-
-
Notifications
You must be signed in to change notification settings - Fork 113
Ensure terminal pane shell location correct when opened #1574
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
base: master
Are you sure you want to change the base?
Conversation
src/FolderManager/FileView.vala
Outdated
set_active_project (path); | ||
} | ||
|
||
private void set_active_project (string path) { | ||
toplevel_action_group.activate_action ( | ||
MainWindow.ACTION_SET_ACTIVE_PROJECT, | ||
new Variant.string (path) | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems recursive no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think so; there are two actions named ACTION_SET_ACTIVE_PROJECT
but with different prefixes. The FileView
one activates the MainWindow
one as well as doing some local stuff. I'll check and see if this can be clarified.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oof yeah, what's the reason for having two actions? Why can't we call the same one always?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The MainWindow action is called by two different FileView actions (ACTION_CLOSE_OTHER_PROJECTS and ACTION_SET_ACTIVE_PROJECT) and there is some pre- and post- processing specific to FileView involved so not sure if that's possible. I could change the name of the FileView action to avoid confusion I guess.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have now lost the FileView action altogether and call the MainWindow one directly. Need to check for regressions but seems OK. There didn't seem a reason to re-write the opened-folders setting on changing active project and this way should improve consistency of behaviour.
Fixes #1523
Consistency of behaviour was improved by using the same MainWindow action to set the active project whether from the project chooser or the context menu and dropping a signal/handler that did much the same thing. Responsibility for updating the git_manager and collapsing the other projects transferred to MainWindow.
Note: To test you must manually set the active project and close/reopen the app in order to ensure the settings are updated.