[00:52] <OvenWerks> teward: yes that works. I like that it is much easier to read and easy to make sure there are spaces where they need to be without adding +" "+ kinds of junk.
[00:53] <OvenWerks> I have found another bug though :(  I will look at fixing that.
[01:05] <teward> yep.  this is why I like format strings :P
[01:05] <teward> OvenWerks: and what bug, if I may ask?
[01:29] <OvenWerks> the visible check box was not working right (just pushed fix) I could make something not visible, but could not make it visible again. And I had to do an edit to do even that.
[01:31] <OvenWerks> teward: ^^ So I have fixed that. (running glade to add one small change sure makes for a big diff)
[01:34] <teward> OvenWerks: I take it that was not my fault then :p
[01:34] <OvenWerks> no not your falt
[01:37] <Eickmeyer> OvenWerks: I just triggered a rebuild.
[01:47] <teward> Eickmeyer: might be a while :Po
[01:47] <teward> I can fast build if you want
[01:48] <Eickmeyer> teward: I'm in no rush.
[01:48] <Eickmeyer> Dunno about OvenWerks
[01:49] <teward> well this also lets me notice any odd new lintian warns :p
[01:49] <Eickmeyer> teward: Also, looks like it's already built and just needs to be publish.
[01:49] <Eickmeyer> teward: True. Go ahead.
[01:50] <teward> yep looks good.
[01:50] <teward> not sure if hte publisher will be fast enough :P
[01:50] <teward> but i'm impatient :)
[01:51] <teward> https://people.ubuntu.com/~teward/ubuntustudio-menu-add_0.1_all_v4.deb if you are as impatient as I :)
[01:51] <teward> glad to hear though my little change there didn't blow it up :)
[01:51] <teward> i thought for a second I had introduced another bug xD
[01:53] <Eickmeyer> Nah, looks like you're good. Any problems that came from that have been circumvented, I take it?
[01:54] <teward> Eickmeyer: AIUI yes
[01:54] <teward> OW said there was a bug in the visible checkbox not working right
[01:54] <teward> but not related to my code
[01:54] <teward> and I think OW is now in love with format strings xD
[01:54] <Eickmeyer> I see that. Looks like he fixed it.
[01:54] <Eickmeyer> hehehehe
[01:54] <teward> both f-strings
[01:54] <teward> and "".format :P
[01:55] <Eickmeyer> Maybe someday I'll learn Python and know what that means.
[01:56] <teward> heh
[02:10] <teward> Eickmeyer: basically?
[02:10] <Eickmeyer> Huh?
[02:10] <teward> instead of doing "some string "+variable+" was here" it's easier to know where things are in the string
[02:11] <teward> f"some string {variable} was here"  <-- easier to make sure things are exactly where they need to be
[02:11] <teward> AND to quote out entire args
[02:11] <Eickmeyer> Okay, that makes sense.
[02:11] <Eickmeyer> I had to turn that into a bash script in my head.
[02:12] <teward> it's not actaully that far off ;)
[14:51] <OvenWerks> Eickmeyer: f-strings are like super bash instring variables as the "variables" can be equasions or the return of functions
[16:17] <teward> OvenWerks: can we confirm the bugs you ID'd are fixed and documented in the changelog and 'ready for upload'?
[16:17] <teward> and that 'this works'?
[16:17] <teward> once that's ready I can responsor it
[16:57] <OvenWerks> teward: they bugs are fixed... running my local file before uploading. I will DL the package and check this afternoon to check that way too.
[16:58] <OvenWerks> teward: in many ways I would like other people to try it as that exposes my blind spots...
[16:59] <OvenWerks> but we seems to have very few people willing to do that.
[17:00] <OvenWerks> When I build something, everything inside is "intuitive" to me... but I tend to be the odd one out in many of these things
[17:12] <teward> OvenWerks: well I don't run Studio
[17:13] <teward> so make Eickmeyer test it :p
[17:13]  * teward is just here as a 'helper' today
[17:25]  * Eickmeyer tested it already
[17:26] <Eickmeyer> teward: I'd go ahead. If we can get it into Eoan, I should have upload rights for bugfixes, or at least can get them from the DMB.
[17:27] <Eickmeyer> If we can get people testing the dailies, that would be the best route at this point.
[17:27]  * Eickmeyer is going crazy applying for jobs.
[17:44] <teward> ack
[17:44] <teward> i will pull latest, debuild, and upload again.  If it rejects I'll let you know.
[17:44] <Eickmeyer> teward: ack
[17:47] <teward> this is missing the subprocess patches...
[17:47] <teward> the one that switches the subprocess call to array based
[17:47] <teward> OvenWerks: did that get pushed?
[17:47] <teward> or did we expect me to apply that?
[17:48] <teward> or did that still not work?
[17:50] <teward> the change which does subprocess.run(["/usr/bin/exo-desktop-item-edit", str(self.tempfile)], shell=False) instead of the string based exec
[17:50] <teward> Eickmeyer: ^ this was the last bit that sarnold wanted done
[17:50] <teward> but it was untested
[17:50] <teward> 'cause i'm not on eoan
[17:50] <teward> nor STudio
[17:51] <teward> so if this still doesn't work i'll be happy to leave it as is with the strings
[17:51] <teward> but sarnold wanted it array-based preferably
[17:51] <teward> rather than string-driven
[17:51] <Eickmeyer> OvenWerks needs to address that then.
[18:11] <teward> ack
[18:16] <OvenWerks> I thought that had been changed. sorry, I will look at it.
[18:17] <OvenWerks>  I thought that was what I tested.
[18:19] <OvenWerks> https://git.launchpad.net/ubuntustudio-menu-add/tree/usr/bin/ubuntustudio-menu-add?id=bd332ecd04cf778bd60208dbd187dd9658eaf7b4
[18:20] <Eickmeyer> teward: ^
[18:20] <OvenWerks> line 418
[18:20] <teward> OvenWerks: that's the format-string driven one
[18:20] <teward> i had another patch hang on
[18:20] <teward> that applied it as an *array* with the full path
[18:20] <teward> guess it never pushed
[18:20] <teward> 'cause i didn't hit push
[18:21] <OvenWerks> I notice the terminal =true too :(
[18:21] <teward> yeah that's why it's not got my code in it.  Mine uses shell=False
[18:21] <teward> and sarnold doesn't like shell=True if it can be avoided
[18:21] <teward> for security reasons
[18:22] <teward> okay...
[18:22] <teward> https://git.launchpad.net/ubuntustudio-menu-add/commit/?id=50791c079560173736ec8eaaa2dc4362b297c0e0
[18:22] <teward> which uses the array
[18:22] <teward> lets see if *this* works.  if not we'll revert
[18:22] <teward> this pulls the full path in so
[18:22] <teward> if this works as-is then all is good
[18:24] <teward> it SHOULD but it'll need a test ;)
[18:27] <OvenWerks> teward: that works here.... so what is different? When we tried it before it failed. Maybe shell == yes needs string?
[18:27] <teward> OvenWerks: no
[18:27] <teward> it's shell=False
[18:27] <teward> taht's what we needed
[18:27] <teward> which i discovered during some tests
[18:27] <OvenWerks> Yes, I agree
[18:28] <OvenWerks> I was wondering why it failed before (yesterday)
[18:28] <teward> yeah i was headscratching
[18:28] <teward> then i did some local Python subprocess call tests
[18:28] <teward> and found it
[18:33] <OvenWerks> The only differences I see are " rather than ', full path and Shell=false
[18:34] <OvenWerks> I think the shell=False could be omitted as being default but leaving it in is self documenting.
[18:34] <OvenWerks> teward: ^^
[18:35] <teward> yeah I left it in there for explicitly making sure.  I used double quotes by accident, but from Python's perspective 'foobar' == "foobar"  as they're both strings
[18:35] <teward> if you prefer single quotes that's a minor change
[18:35] <OvenWerks> Not worth it
[18:35] <teward> mmkay.  I need to add some changelog entries since I contributed now :|
[18:36] <OvenWerks> I tend toward double quotes... not sure why (K&R c maybe?)
[18:36] <teward> *SHRUGS*
[18:36] <teward> oops caps
[18:38] <teward> OK so I also added to the changelog since it has my two revisions per sarnold
[18:38] <teward> sarnold: want to do one last review before this gets reeval'd for sponsoring?
[18:39] <sarnold> sure
[18:40] <teward> https://git.launchpad.net/ubuntustudio-menu-add/tree/usr/bin/ubuntustudio-menu-add should be less offensive wrt subprocess and lib calls :)
[18:43] <sarnold> btw is there a reason why this line uses concatenation?
[18:43] <sarnold> self.tempfile = str(os.path.expanduser("~/"))+".config/ubuntustudio_menu_temp.desktop"
[18:43] <teward> sarnold: because OvenWerks hasn't gotten to using an f-string for it yet
[18:43] <sarnold> hmm I guess I could just try ..
[18:43] <teward> since they only learned about format strings yesterday-ish(?)
[18:44] <sarnold> os.path.expanduser("~/.does/not/exist")
[18:44] <sarnold> '/home/sarnold/.does/not/exist'
[18:44] <sarnold> no need for foramt strings
[18:44] <teward> sarnold: the concat is to add the rest.
[18:44] <teward> that's why the concat as is works
[18:45] <teward> and yes i'm annoying asking sarnold to spotcheck
[18:45] <teward> but since sarnold caught the first batch of errors
[18:45] <teward> i'm being cautious :)
[18:46] <teward> oh OvenWerks just a question
[18:46] <teward> self.menufolder in menu_cb
[18:46] <teward> that's not init'd in __init__
[18:46] <teward> should it be?
[18:46] <teward> even if it's init'd to None?
[18:46] <teward> (code inspection shows it as a low-level consideration)
[18:46] <teward> (not blocking but still a question)
[18:48] <sarnold> teward: well, it looks like os.path.expanduser() doesn't need the file to exist, it should work fine without the concatenation or format string or hwatever
[18:48] <teward> indeed, it doesn't.
[18:48] <teward> sarnold: so you're saying straight os.path.expanduser("~/.config/ubuntustudio_menu_temp.desktop")  ?
[18:48] <teward> (as an example)
[18:49] <sarnold> yeah that should work, right?
[18:50] <teward> yep even without the str-casting since it returns a str
[18:50] <sarnold> yay
[18:51] <teward> anything else glaring?
[18:51] <teward> Eickmeyer: OvenWerks: and forgive our bootprints over this :)
[18:52] <sarnold> nope, thanks for fixing up the executions to use arrays :)
[18:52] <teward> yeah that was a PITA to trace
[18:53] <teward> OvenWerks: at your convenience, `git pull` and test please :)
[18:53] <teward> if all still works
[18:53] <teward> i'll sponsor it up :)
[18:57] <Eickmeyer> teward, sarnold: I think I speak for both of us when I say any improvements are welcoe.
[18:57] <Eickmeyer> *welcome
[18:57] <teward> indeed.
[18:57] <teward> Eickmeyer: of course, sarnold does A LOT of code review so spots things I sometimes don't :)
[18:57] <Eickmeyer> Makes sense.
[18:58] <sarnold> everyone spots different things :)
[18:58] <sarnold> I'm good at noticing things along syscall boundaries
[18:58] <sarnold> and sometimes algorithmic mistakes
[18:58] <sarnold> other people know specific idioms in languages better
[18:59] <OvenWerks> teward: self.menufolder is actually initialized in the glade file.
[18:59] <sarnold> but now time to watch some women's world cup :) have fun
 OvenWerks: ah.  ack, OK.
 just thought I'd ask :)
 OvenWerks: if everything works fine here though as-is then this is Good 2 Go and I'll be happy to upload (unless you want a test build too)
[18:59] <OvenWerks> the data is static unlike submenu
[19:00] <OvenWerks> I am still testing, I looked at the init first
[19:03] <OvenWerks> I din't know I could give the whole string for expanduser. Thank you. Yes that still works.
[19:04] <OvenWerks> teward: thinking about logically, it does make sense I could do it that way.
[19:05]  * OvenWerks wonders how ubuntustudio-controls got through...
[19:05] <Eickmeyer> OvenWerks: Probably because it was an already-existing binary.
[19:07] <OvenWerks> I will go through -controls sometime fixing some of these things there as well.
[19:08] <OvenWerks> The lines of code count should go down.
[19:08] <teward> OvenWerks: probably because it didn't have me or sarnold digging into the code :P
[19:08] <teward> (my coredev is new as of this week and Eickmeyer asked me to poke so :P)
[19:09] <OvenWerks> teward: I will at least go through the code first before we sak for a look.
 👍
[19:10] <OvenWerks> I should be able to get rid of things like shell=True
[19:10] <teward> OvenWerks: yeah, since shell=True is already insecure
[19:10] <teward> they even say it in the docs xD
[19:10] <teward> thanks for the effort though, just make sure it works with the array execution and those strings being expanded
[19:10] <teward> if all looks good I'll kick it into NEW queue
[19:11] <OvenWerks> yup, but it is obvious to me my main problem there is using the shell to find the executable, just give the path and that can go away
[19:11] <OvenWerks> teward: ubuntustudio-menu-add looks good to me. anything those changes affect has been tested.
 ack
 are you and Eickmeyer comfortable with this being 0.1 in the repos then?
[19:12] <OvenWerks> yes
[19:13] <OvenWerks> when I work on -controls, I will commit each kind of change. (maybe tab to space frst)
 good idea :)
[19:14]  * OvenWerks is off to lunch
[20:49] <teward> OvenWerks: Eickmeyer: just as an FYI: the package was accepted to the NEW queue as is now
[20:49] <teward> it's waiting AA review
[20:49] <teward> at their convenience.
[20:50]  * teward goes to pack up before heading home from work