Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added an option to remember the path were the last file was saved #3001

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

gustavo-silva-serra
Copy link

Hey, I have never done this before so I don't know what I should write in here. I hope I haven't done anything wrong.

I tried to address request #2894 so Geany can remember where the last file was saved.

@xiota
Copy link
Contributor

xiota commented Nov 15, 2021

In utils.c, why is the previously saved path after the project path? For someone who almost always has a project open, it will look like the new option does nothing.

@xiota
Copy link
Contributor

xiota commented Nov 15, 2021

"Opens save file dialog in the same directory" – Based only on this description, it's not clear what's supposed to happen. Maybe use "Default to last save directory" as a starting point, as suggested elsewhere.

@gustavo-silva-serra
Copy link
Author

Hi! Thank you for reviewing.

I will change the option "Default to last save directory". I forgot that the correct way to describe the option was already there. Since I am not fluent it is hard for me to express some ideas.

I was not sure that this new option should precede the project path because I see the project options coming first in a hierarchy of options. I don't use projects myself, so I don't know how this option should interact. Anyway, I can make the change no problem if it is best.

This is my first pull request ever. Should I make a new one with the adjustments or this pull request can be updated with the new code?

Thank you again.

@kugel-
Copy link
Member

kugel- commented Nov 15, 2021

Does anyone know why this should be guarded with a pref? Can't we just always do it?

Plus, I think it makes sense to store this in session.conf (see https://github.com/geany/geany/tree/session_split) which I plan to merge for 1.39/2.0

@gustavo-silva-serra
Copy link
Author

I think it makes sense because maybe the users are used to navigating through console to a folder and opening Geany there.

I am new here, I don't know how these things work. Can anyone point me to what exactly I must do? I was planning to fix the option description and try to submit the pull request again.

@gustavo-silva-serra
Copy link
Author

I have fixed the option description. Is there anything else I should do?

Thanks in advance.

@xiota
Copy link
Contributor

xiota commented Nov 16, 2021

I have no say in what happens... but what basically happens is you just hope a dev is interested and make changes as requested until they think it's good enough to merge.

What's the current path use order vs the new path use order? Does this PR save the path between restarts, or only reuse the path during the same session? (I wouldn't want the path to carry over between restarts, but some devs might.)

@kugel-
Copy link
Member

kugel- commented Nov 16, 2021

I don't feel strong whether it's remembered across restarts or not but I do think it should be applied before the project path.

@elextr
Copy link
Member

elextr commented Nov 16, 2021

As its a setting in preferences it should be remembered like all the rest of the settings in preferences.

Agree that it needs to be applied before the project path, using the project path is the fallback if this is not set.

@xiota
Copy link
Contributor

xiota commented Nov 16, 2021

Currently, based on observation, not examination of code, it looks like the path from the current file is used. If the current file has no path (untitled), the project path is used. If there is no project path, the home folder is used. So an existing "workaround" to use the last used save-as path is just to not close the last saved file or keep a project file open.

I do think it should be applied before the project path. (@kugel-)

Agree that it needs to be applied before the project path, using the project path is the fallback if this is not set. (@elextr)

When working in a project, I open and save non-project files because it's inconvenient to unload and reload projects. If the save-as path is within the project, it's reasonable to use it instead of the project path. But when going back to editing project files after working on a single out-of-project file, does it really make sense for save as to override the project path with outside project paths?

@elextr
Copy link
Member

elextr commented Nov 16, 2021

Currently, based on observation, not examination of code, it looks like the path from the current file is used. If the current file has no path (untitled), the project path is used. If there is no project path, the home folder is used. So an existing "workaround" to use the last used save-as path is just to not close the last saved file or keep a project file open.

Better check the code (I haven't yet either) because it doesn't make sense for this to apply to anything that already has a path, should use that as the starting point for save-as, and using the current file when it does not have a path doesn't make sense either.

When working in a project, I open and save non-project files because it's inconvenient to unload and reload projects.

Ditto. Thats really the problem with this being a preference and applying to all saves, the user wants to use a project path if its a new project file, conversely using the project is wrong for the non-project files. Can't win ☹️

@xiota
Copy link
Contributor

xiota commented Nov 16, 2021

What about:

  • If no project is loaded, and the previous save-as path is valid, use it instead of the home folder. Otherwise, use the home folder.

  • If a project is loaded, don't use the previous save-as path unless it is a subdir of the project folder or the project folder is invalid. If the previous save-as path is also invalid, fallback to the home folder.

@gustavo-silva-serra
Copy link
Author

What about:

  • If no project is loaded, and the previous save-as path is valid, use it instead of the home folder. Otherwise, use the home folder.
    If I understood you right, this is what I originally implemented. The project path has precedence over the last save-as path.
  • If a project is loaded, don't use the previous save-as path unless it is a subdir of the project folder or the project folder is invalid. If the previous save-as path is also invalid, fallback to the home folder.
    I think that this is more behavior than a simple checkbox should allow.

IMO if a user creates and opens a project, he wants to work inside the scope of that project, including its options. In the worst case, the users are already used to how the project path option works. That is why I think we should not change the behavior of something that is already in production for so long (even considering that the user would intentionally check de option).
I don't mind fixing the precedence so the last save path is used before anything else, I just think that the behavior should be as simple and assertive as possible.
Finally, the option could be a select box (I don't know how you guys call it... drop down box?) and let the user choose the behavior:
always the last path: override project
project path: project path first, last path second
home: user home folder
default (current): project, that option to set a default path or where geany is running from (as it is today).

@elextr
Copy link
Member

elextr commented Nov 16, 2021

Let me summarise Geany current behaviour without this change.

if a project is open and it has a base directory: 
    use that as filechooser directory and also add a shortcut to the chooser
else if a "startup path" value is set in prefs: use that as filechooser directory

If the document has a path
    if the path is an absolute path: 
        Geany uses that path and directory in the file chooser overwriting any directory setting made above
    else: Geany sets the filename and keeps whatever directory (if any) was set above
if no directory has been set the dialog will fallback to GTK default behaviour (whatever that is)

Currently Geany does not set home, or current working directory, it falls back to the GTK filechooser default behaviour.

I'm not sure what the GTK default behaviour is supposed to be, it may be distro/desktop dependent, here it often seems to default to the current working directory, not home, but even if its not well defined I expect its best to keep using that so Geany follows the normal filechooser behaviour like other apps, not force to home. Note that home is always a shortcut on the filechooser so its easy to select.

I have no idea what the Windows native dialog does.

I am presuming @xiota's suggestions below only apply if the document has no existing path and that "previous save-as path" is the path the last time the save-as dialog was used, not the path of the last saved file.

If no project is loaded, and the previous save-as path is valid, use it instead of the home folder. Otherwise, use the home folder.

Makes sense except should fallback to the prefs "startup path" if there is no previous save-as path then to GTK default behaviour not home.

If a project is loaded, don't use the previous save-as path unless it is a subdir of the project folder or the project folder is invalid. If the previous save-as path is also invalid, fallback to the home folder.

Makes sense except fallback to GTK default behaviour not home.

Note that if valid the project directory is added as an additional shortcut to the ones that the filechooser shows normally. Perhaps a valid last path should be too.

Of course shortcuts can also be defined by the user using the desktop external to Geany, I have a "data" folder pointing to my mirrored array.

@xiota
Copy link
Contributor

xiota commented Nov 17, 2021

I am presuming @xiota's suggestions below only apply if the document has no existing path and that "previous save-as path" is the path the last time the save-as dialog was used, not the path of the last saved file.

Yes.

Makes sense except should fallback to the prefs "startup path" if there is no previous save-as path then to GTK default behaviour not home.

On my computer, the "startup path" is blank, and the GTK default is probably the home folder. So whenever I refer to "home", substitute with "startup path" and GTK default.

@elextr
Copy link
Member

elextr commented Nov 17, 2021

A couple of reference points of other GTK editors, both Xed and Gedit use home as their save-as directory when started from the menu, and the current working directory if started from the command line. I don't have a Geany installed as system so I can't try using the menu, but from the command line it follows the working directory (on Linux, IIRC Windows has some other working directory weirdness).

If save-as has been used this session Xed will use that directory for any subsequent save-as, it will not use the directory of any files just saved. Basically thats the same as the project-less proposal above.

Neither Xed or Gedit have anything like projects, so can't help there.

@xiota
Copy link
Contributor

xiota commented Nov 17, 2021

I don't have a Geany installed as system so I can't try using the menu, but from the command line it follows the working directory...

I usually run Geany from a shortcut. Running from command-line does follows PWD.

If the remembered save-as folder is just put in the right place, along with checks to account for projects, shouldn't the existing fallback behavior pretty much remain intact? (So instead of "home", I should have referred to "the rest of the existing fallback".)

@gustavo-silva-serra
Copy link
Author

How are you people?

Due to personal reasons, I won't be able to continue with this feature. What should I do? Do I just leave this pull request open? Do I close it?
Thank you for your time and sorry for any inconvenience.

@xiota
Copy link
Contributor

xiota commented Nov 19, 2021

Due to personal reasons, I won't be able to continue with this feature. What should I do? Do I just leave this pull request open? Do I close it?

@gustavo-silva-serra It looks like a couple devs are interested in this, so if you were to continue with it, there's a good chance it could be merged eventually. I don't know what the "right" thing to do is. If you decide to close it, you can still reopen it later if you decide to continue working on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants