edgetx icon indicating copy to clipboard operation
edgetx copied to clipboard

Allow the last used directory when opening a file to be saved in the Compaion config file

Open pagrey opened this issue 10 months ago • 5 comments

Is there an existing issue for this feature request?

  • [X] I have searched the existing issues

Is your feature request related to a problem?

I have noticed Companion doesn't write the lastDir entry anymore so File>Open defaults to the main EdgeTX folder. It seems that my entry had carried over from previous config files as I have updated Companion (possibly back as far as OpenTX). Starting with no previous config file the entry isn't created. Manually editing the config file and adding/editing lastDir= works. It would be nice for the lastDir entry to be written so you can easily open files in previous directory. The lastDir entry seems tied to eepromDir but there isn't really an eeprom folder in the settings.

Describe the solution you'd like

This simple change seems to work for me and lastDir is created and updated in the config file. https://github.com/EdgeTX/edgetx/compare/main...pagrey:edgetx:save-dir

I'm really just curious why openFile always sets updateLastUsedDir false, as far back as I looked it has been false. I gave up when I started seeing OpenTX. I don't consider this a bug, it seems to work as it is currently written.

Describe alternatives you've considered

No response

Additional context

No response

pagrey avatar Mar 26 '24 20:03 pagrey

Interesting... that line hasn't been changed in ... seven years... so certainly not a recent... "change" :zany_face:

image

Seems it was changed/added while fixing a drag and drop issue: "Enable .otx file drag&drop on main window"

cf. https://github.com/opentx/opentx/commit/0a351112dd001dd0cd36d6e40017bd0b14dcd0a0 and https://github.com/opentx/opentx/issues/4224

pfeerick avatar Mar 27 '24 00:03 pfeerick

Curious, the filename is now passed to the openFile method but the line g.eepromDir(QFileInfo(fileName).dir().absolutePath()); is essentially removed because false is alwyas passed to the method. I don't know why the file name had to be passed to the method but since the new method does the same check if (!fileName.isEmpty()) Seems like the fix would be to dump the boolean which doesn't do anything. I think removing it and the associated if (updateLastUsedDir) would work. The isEmpty already checks that you have a good value to update. I'm not suggesting to change it but that boolean doesn't do anything now other than comment out a single line of code that updates the file path.

I'm going to make the fix on my fork so the boolean is eliminated and the associated check so the eepromDir is updated. Thanks for tracking down the change, I didn't know how to do it.

pagrey avatar Mar 27 '24 03:03 pagrey

Using my screenshot as a reference, on GitHub, there is a button up the top that says "Blame" (right under the left pointing arrow). Normally the "Code" buttons is selected. When you hit the "Blame" button, it updates to the view shown there, and you can see when lines were last edited, and jump to the relevant commits. If you want to tunnel back in time further for a particular line, you click on the share icon with a ... trail behind it? ... on that line, and it jumps further back.

Looking forward to a PR clearing that up ;)

pfeerick avatar Mar 27 '24 05:03 pfeerick

@pagrey I see it has been a couple of months since you advised you would submit a PR to fix this. Are you still planning to do so or would you like for me to action?

elecpower avatar Jun 11 '24 21:06 elecpower

Since it has come up I have created a pull request #5161 with my very simple fix. @elecpower please take any action you would like to, this is just my basic fix for the issue.

pagrey avatar Jun 12 '24 00:06 pagrey