Improvements to Close Others #7038
Conversation
| } | ||
| } | ||
| } | ||
|
|
||
| if (settings.close_below) { |
There was a problem hiding this comment.
I thought about either removing these parts (settings) completely or replacing them with the new preferences system, but I wanted to hear your opinion first.
There was a problem hiding this comment.
I was thinking the same. I think it might be cool to use the preference system, since this settings seem hard to edit.
|
I just added another commit to address these nits and use the PreferencesManager instead of the settings file. |
| working_set_cmenu.addMenuItem(close_below, "", Menus.AFTER, Commands.FILE_CLOSE); | ||
| function prefChangeHandler() { | ||
| // it's senseless to look prefs up for the current file, instead look them up for the current project | ||
| var enabled = prefs.get("enabled", PreferencesManager.CURRENT_PROJECT), |
There was a problem hiding this comment.
I don't think that we need an additional enabled pref. It doesn't make much sense to have the menu items, but the commands disabled. If you want to disable them, you can just disable all the other menus.
There was a problem hiding this comment.
It shouldn't ever happen to have the menu entries but not the commands (we need the commands to register menu entries). I added this pref to make disabling the whole thing easier.
But I can remove it if you want me to.
There was a problem hiding this comment.
Right... i still think is not needed since is just 3 preference to mark as false, which is not that much code:
{
"closeAbove": false,
"closeBelow": false,
"closeOthers": false
}
There was a problem hiding this comment.
Actually it is:
{
"closeOthers.closeAbove": false,
"closeOthers.closeBelow": false,
"closeOthers.closeOthers": false
}
Do you think like this it might be better?
{
"closeOthers.above": false,
"closeOthers.below": false,
"closeOthers.others": false
}
There was a problem hiding this comment.
Yes, that's better. Changed, thanks.
|
Adressed all comments. |
|
It looks good. Will wait and see if @JeffryBooher has any input for this. |
|
Looks good to me...merging |
Improvements to Close Others
As mentioned in #6020 (comment), I just did some improvements to the Close Others extension:
DocumentCommandHandlersmodule - it wasn't usedbeforeContextMenuOpenUnit tests still passing.
@JeffryBooher