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

Add option to prevent reloading projects that are already loaded #2951

Closed
wants to merge 2 commits into from

Conversation

xiota
Copy link
Contributor

@xiota xiota commented Oct 24, 2021

This PR adds an option (projects.reload_already_open) to control whether Geany reloads projects that are already loaded.

  • The real paths are compared so symlinked files are also caught.
  • This affects files opened from both the command line and UI.
  • The default is false, which prevents projects from being reloaded. Although this is a change from the current behavior:
    • Review of existing plugins indicates minimal, if any, negative effect.
    • The most noticeable difference will be a reduction of warning popups. Another effect is that undo history will not be lost by an unnecessary reload, which potentially prevents data loss.
    • The option can be enabled by those who really want it.
  • When a project is not reloaded, a message is sent to the status window to notify the user.

Copy link
Member

@elextr elextr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this needed? What are the downsides of reloading projects? Also does reloading a project have useful semantics that this would block, I'm not sure?

If this is desired it should be applied to UI activated opening as well, by doing this in the project open code, not socket code.

Also technically it should be comparing realpathed absolute filenames so it won't refuse to load projects of the same name in different directories.

src/socket.c Outdated Show resolved Hide resolved
@xiota
Copy link
Contributor Author

xiota commented Oct 24, 2021

Is this needed?

Yes.

What are the downsides of reloading projects?

  • Popup to warn about closing project – waste time closing popup.
  • Geany doesn't take focus when it creates popups ­– waste time switching to Geany when it would otherwise take focus.
  • Popups to save unsaved files – more wasted time. And they're annoying.
  • Lost undo history.

Also does reloading a project have useful semantics that this would block, I'm not sure?

Nothing useful. It creates lots of popups and destroys undo history. Because of the popups, it's faster to reopen a project by closing and rerunning Geany itself.

If this is desired it should be applied to UI activated opening as well, by doing this in the project open code, not socket code.

I made the change here because that is where the function to generate the popup is called. That function does not receive the new project file name, so cannot make a comparison.

When a project is opened from the menu, no warning is generated. So even if the project were prevented from being opened elsewhere, the change here would still be necessary to prevent the warning dialog from being shown.

Also technically it should be comparing realpathed absolute filenames so it won't refuse to load projects of the same name in different directories.

app->project->file_name contains the full path. The GeanyProject struct does not appear to have a real_path component.

@elextr
Copy link
Member

elextr commented Oct 24, 2021

Popup to warn about closing project – waste time closing popup.

#2949 means you can avoid this if you don't want it 😄

Geany doesn't take focus when it creates popups ­– waste time switching to Geany when it would otherwise take focus.

IIRC all Geany popups are marked modal, so they should take focus, but it is up to the window manager/wayland compositor to grant that. Seems to work here with Cinnamon.

Nothing useful.

Ok, but project load does cause a signal that plugins might use, do the project plugins do rescans or other stuff when that happens.

When a project is opened from the menu, no warning is generated.

Oh, Ok, well no need to add one then, coulda sworn it did, but maybe its been removed, maybe github search knows ... ahh, it used to for a "recent project" open, but that was removed in f40569d.

The GeanyProject struct does not appear to have a real_path component.

Yeah, I said "technically", but absolute should be fine most of the time to distinguish two files of the same name in differing directories, only getting confused for smarties with multiple paths to the same file, but in that case its no different to the current situation, so no need for real_path.

app->project->file_name contains the full path.

Do you get an absolute path from the socket buffer then?

BTW I am not saying this PR is a bad idea, just want to be sure the implementation has no negative side effects.

@xiota
Copy link
Contributor Author

xiota commented Oct 25, 2021

#2949 means you can avoid this if you don't want it smile

Haha... yes...

IIRC all Geany popups are marked modal, so they should take focus, but it is up to the window manager/wayland compositor to grant that. Seems to work here with Cinnamon.

It's probably focus stealing prevention... But I prefer no popups in the first place.

Ok, but project load does cause a signal that plugins might use, do the project plugins do rescans or other stuff when that happens.

Since the project is already open, I'd expect that plugins would have done what they needed the first time the project was opened. I used grep to look for plugins that handle the project-close and project-open signals.

  • geanylua ­and geanypy – pass the signals on to scripts.

  • projectorganizer – close/open/update project, update sidebar. I don't see anything that looks like updating indices in response to open/close signals, but those could be hiding in function calls I didn't follow (I only looked at only 2-3 levels across two files). I've been using this plugin and PR together with no ill effects.

  • scope – Functions that call functions that call functions that look like they eventually update the menu. Since it's the same project, I would guess the menu state would end up in the same place.

  • geanyctags – Changes widget sensitivity. Would not be affected by this PR because it's clearly supposed to end up in the same state.

  • treebrowser – Updates the path. There are also buttons and context menu items that can be used to update the path to achieve similar effect.

Surprisingly, workbench does not respond to any project-based signals. It uses only document-open and document-close.

When a project is opened from the menu, no warning is generated.

Oh, Ok, well no need to add one then, coulda sworn it did, but maybe its been removed, maybe github search knows ... ahh, it used to for a "recent project" open, but that was removed in f40569d.

My main concern was opening from the file manager, since that's how I've been opening projects. Maybe already-open projects can continue to be re-opened from the menu because "Unlikely to be done accidentally." ?

Or the behavior of this PR could be put behind an option, like files.reload_project_open_same ?

... only getting confused for smarties with multiple paths to the same file, but in that case its no different to the current situation, so no need for real_path.

I missed the symlink issue. I've changed it to use the real path.

Do you get an absolute path from the socket buffer then?

I think so because the filenames are able to match, preventing the project from opening.

BTW I am not saying this PR is a bad idea, just want to be sure the implementation has no negative side effects.

My interpretation is that your intention is to improve the PR... Or if it's really misguided, help me figure it out...

@xiota
Copy link
Contributor Author

xiota commented Oct 25, 2021

I put the behavior from this PR behind an option in various, "projects.reload_already_open". When a project is prevented from reloading, a message is sent to the status window. I've updated the initial description.

@xiota xiota changed the title Don't reopen project files that are already open Add option to prevent reloading projects that are already loaded Oct 25, 2021
@xiota xiota requested a review from elextr October 25, 2021 12:38
@xiota
Copy link
Contributor Author

xiota commented Nov 7, 2021

Force push to rebase and combine commits.

@xiota
Copy link
Contributor Author

xiota commented Nov 22, 2021

Force push to rebase and guarantee no conflicts with recent session-split commit.

@xiota xiota closed this Apr 25, 2022
@xiota xiota deleted the pr-project-no-reopen branch April 25, 2022 11:38
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

2 participants