[OPEN] "close others" extension added to default extensions directory. #4469
Conversation
| "close_others": "true", | ||
| "close_above": "false", | ||
| "close_below": "false" | ||
| } |
There was a problem hiding this comment.
By default i have enabled "Close Others" only. If user wish they can enable other two options.
There was a problem hiding this comment.
I say we enable them all then let the user turn them off if they wish.
There was a problem hiding this comment.
These settings will be hard to change, since these are saved in the core code, which is replaced with each update. And working with git, it will show this as file that was modified.
Would it be ok to have these preferences as menu items? Or we should leave them like this until the Preference Dialog is done, so we can add them there? Having submenus will really help for this things.
There was a problem hiding this comment.
Next i planned to add "Close Others", "Close Others Above", "Close Others Below" as dynamically, based on user selection. I am not sure, whether there is an option to add context menus dynamically. If i succeed with that, then there won't be settings.json. Until i feel it is good to be there. Most of the users don't use settings.json, until they know it.
ghost
assigned
| define(function (require, exports, module) { | ||
| "use strict"; | ||
|
|
||
| var Menus = brackets.getModule("command/Menus"); |
There was a problem hiding this comment.
minor nit but this should be 1 var statement separated with comas.
var Menus = ...,
CommandManager = ...,
Commands = ...,
etc...
|
done with initial review |
|
@JeffryBooher I did code update. Let me know your thoughts on this. |
| @@ -0,0 +1,57 @@ | |||
| /*jslint vars: true, plusplus: true, devel: true, nomen: true, regexp: true, indent: 4, maxerr: 50 */ | |||
There was a problem hiding this comment.
Should this file have the Adobe license since it will be part of the core now?
There was a problem hiding this comment.
seems true. all other extensions holding Adobe License. Once i get feedback from @JeffryBooher for coding standard, and if he say any other changes, i'll give next commit with Adobe License update.
There was a problem hiding this comment.
Sure. Every non thirdparty js file has it. Make sure to use the year 2013 when adding it.
|
For any of these commands -- I am prompted to save all changes in all files. I shouldn't be prompted to save the file I'm not closing. |
|
Would you accept if i do code changes in the core (DocumentCommandHandler and DocumentManager) for this? |
|
@sathyamoorthi if you need to make changes to those files to support the extension then, yes, we would take them. |
|
@JeffryBooher created another pull request so closing this. |
#4004