Add "Show in OS" context menu item to open a folder in Finder/Explorer. #2128

peterflynn · 2012-11-15T19:30:35Z

Creates a new ProjectManager.getSidebarSelectedItem() API, and renames the existing getSelectedItem() -> getTreeSelectedItem() for clarity.

Create new ProjectManager.getSidebarSelectedItem() API, and rename getSelectedItem() -> getTreeSelectedItem() for clarity.

peterflynn · 2012-11-16T22:26:49Z

src/nls/root/strings.js

@@ -216,6 +216,7 @@ define({
    "CMD_NEXT_DOC"                        : "Next Document",
    "CMD_PREV_DOC"                        : "Previous Document",
    "CMD_SHOW_IN_TREE"                    : "Show in File Tree",
+    "CMD_SHOW_IN_OS"                      : "Show in OS",


Note: I considered being clever and using different strings per OS ("Show in Finder" / "Show in Explorer") but thought I'd try the simplest thing possible first :-)

redmunds · 2013-02-13T00:20:39Z

FYI, there's an extension (https://github.com/ujjaval/Open-containing-folder) that does this, so maybe we don't need this in core?

GarthDB · 2013-02-13T01:08:35Z

I personally think it should be core functionality
—
Sent from Mailbox for iPhone

On Tue, Feb 12, 2013 at 4:21 PM, Randy Edmunds notifications@github.com
wrote:

FYI, there's an extension that does this (https://github.com/ujjaval/Open-containing-folder), so maybe we don't need this in core?

Reply to this email directly or view it on GitHub:
#2128 (comment)

Conflicts: src/command/Menus.js src/document/DocumentCommandHandlers.js

…path - so file can be shown selected

peterflynn · 2013-05-14T00:24:55Z

Now that this has sat for six months (!), I wonder if the breaking ProjectManager API change is no longer a good idea. But I still do think it'd be good to clarify what the API does (it's easily misunderstood today). Is this a good use case for @dangoor's deprecation suggestions? I.e. keep the old API around for a while, but have it print warnings to the console so developers migrate over...

gruehle · 2013-05-14T03:30:33Z

src/command/Menus.js

@@ -980,6 +980,241 @@ define(function (require, exports, module) {
    //     NOT IMPLEMENTED
    // }

+
+    function init() {


This function shouldn't be in here. All of the menu initialization has been moved to DefaultMenus.js.

gruehle · 2013-05-14T03:39:04Z

Do we even need separate functions for getTreeSelectedItem() and getSidebarSelectedItem()? How about just modifying getSelectedItem() to do everything that's in getSidebarSelectedItem().

The only place where we call getSelectedItem() and don't want working set entries is in _handleNewItemInProject(). But, it turns out that the "selected item" value is ignored in that case anyway...

gruehle · 2013-05-14T03:40:01Z

src/document/DocumentCommandHandlers.js

+        var entry = ProjectManager.getSidebarSelectedItem();
+        if (entry) {
+            brackets.app.showOSFolder(entry.fullPath, function (err) {
+                console.error("Error showing '" + entry.fullPath + "' in OS folder:", err);


Should only call console.error() if err is non-zero.

gruehle · 2013-05-14T03:40:55Z

Initial review complete.

…dItem() API, which now covers the working set selection too. Seems like a fairly safe API change - this may even be how people assumed it worked before. Also: add logging for a ProjectManager error case that was silently ignored before.

peterflynn · 2013-05-16T22:01:30Z

@gruehle changes pushed. After some thought I agree with you about the API change -- I think few extensions use getSelectedItem() and if any do, it's just as likely they already were assuming it covered the whole sidebar. I'll add this to Release Notes once it's merged, though. (Also -- the way working set selections get ignored in File > New is a little odd, so I added an extra comment there clarifying).

I also threw in a totally unrelated console.warn() for a ProjectManager error case that was silently ignored up until now... just for kicks.

gruehle · 2013-05-17T02:56:36Z

Looks great! Merging.

Add "Show in OS" context menu item to open a folder in Finder/Explorer.

lkcampbell · 2013-05-18T15:53:24Z

@peterflynn and @gruehle,

OS: Windows 7
Build: sprint 25 development build 0.25.0-0 (master 8653151)

This is not working properly for me. It appears to be opening the file in the default application I have assigned for the given file extension. For example, .txt opens in Notepad and .html opens in Chrome.

As a comparison, Open Containing Folder extension works correctly for me.

WebsiteDeveloper · 2013-05-18T16:10:48Z

@lkcampbell for me it works fine
I'm on Windows 7 Professional

lkcampbell · 2013-05-18T16:19:47Z

@WebsiteDeveloper

Interesting. I wonder what the difference is between you and me? I'm going to investigate the differences between this change and the Open Containing Folder extension. I will probably have some follow up questions for you to figure this out.

lkcampbell · 2013-05-18T16:44:24Z

Okay, I don't know why it works for @WebsiteDeveloper and not me. Maybe we have our Windows Explorer settings set up differently and his settings are more forgiving than mine.

I can, however, tell you how this code's behavior is different from the Open Containing Folder extension on my machine.

Open Containing Folder send the directory path name to brackets.app.showOSFolder():

"C:/Users/Lance/brackets/src/document"

and this code sends the file path name to brackets.app.showOSFolder():

"C:/Users/Lance/brackets/src/document/DocumentManager.js"

RaymondLim · 2013-05-18T17:21:37Z

@lkcampbell You're not alone! I can reproduce it on Win7, but not mac. I'll be trying on winXP and log a new issue soon. On my win7 it only works for folders, but not files.

WebsiteDeveloper · 2013-05-18T17:44:14Z

@lkcampbell it seems that for me the difference is, that when opening a containing folder via brackets the file is selected whereas via extension no file is selected in the Explorer.

RaymondLim · 2013-05-18T18:14:51Z

File a new issue #3903.

@lkcampbell and @WebsiteDeveloper If you're seeing something different, please comment on the differences in #3903.

RaymondLim · 2013-05-18T18:52:35Z

@lkcampbell It turns out that we need the latest shell code to get it working correctly.

lkcampbell · 2013-05-18T19:11:40Z

@WebsiteDeveloper, do you have the lastest shell code? If so, where did you get it? Are you building the shell yourself?

@RaymondLim, to clarify, if I want to retest this, I have to build the shell myself, correct?

I am trying to remember to also include the build version on my issue reports now. Does this version info from the About dialog:

sprint 25 development build 0.25.0-0 (master 8653151)

contain any information about the Shell version?

If not, maybe that should be included in the About dialog box as well. It could have been useful for troubleshooting the problem.

WebsiteDeveloper · 2013-05-18T19:25:13Z

@lkcampbell i built the shell myself yesterday i think. if you would like i could upload a fresh build for windows.
And i agree with you, that it would be usefull if the shell version would be displayed in the about dialog.

lkcampbell · 2013-05-18T19:29:53Z

@WebsiteDeveloper, yeah if you wouldn't mind uploading it that would be helpful. Regardless, I think I will look into setting up my own build of the Shell as well. I have been putting it off for too long :).

I'm going to open an issue (i.e. feature request) on adding the Bracket Shell version to the About dialog as well.

WebsiteDeveloper · 2013-05-18T20:25:55Z

@lkcampbell i uploaded the build i currently use:
https://www.dropbox.com/s/4bql92bv0okst46/Brackets-Sprint25-interim-build.zip

lkcampbell · 2013-05-19T00:22:09Z

@WebsiteDeveloper and @RaymondLim, thanks for your help. I actually got the brackets-shell build working on my computer today so I won't be making errors like this again :).

Add "Show in OS" context menu item to open a folder in Finder/Explorer.

c2cd079

Create new ProjectManager.getSidebarSelectedItem() API, and rename getSelectedItem() -> getTreeSelectedItem() for clarity.

ghost assigned gruehle Nov 16, 2012

peterflynn reviewed Nov 16, 2012
View reviewed changes

peterflynn added 2 commits April 29, 2013 23:47

Merge remote-tracking branch 'origin/master' into pflynn/open-in-os

6cf1b4f

Conflicts: src/command/Menus.js src/document/DocumentCommandHandlers.js

Pass full path of files to showOSFolder() - not just parent folder's …

d3dc96a

…path - so file can be shown selected

peterflynn mentioned this pull request May 14, 2013

APIs to open a folder with file pre-selected adobe/brackets-shell#244

Merged

gruehle reviewed May 14, 2013
View reviewed changes

peterflynn added 2 commits May 16, 2013 11:50

Code review comments: fix bad merge, only log warning for actual errors

45e1607

gruehle added a commit that referenced this pull request May 17, 2013

Merge pull request #2128 from adobe/pflynn/open-in-os

b33d64f

Add "Show in OS" context menu item to open a folder in Finder/Explorer.

gruehle merged commit b33d64f into master May 17, 2013

gruehle deleted the pflynn/open-in-os branch May 17, 2013 02:56

lkcampbell mentioned this pull request May 18, 2013

Feature Request: Add the Brackets Shell version to the About Dialog #3904

Closed

marcelgerber mentioned this pull request Feb 22, 2014

Rename 'Show in OS' to 'Show in Finder' for OS X #6970

Closed

core-ai-bot mentioned this pull request Aug 17, 2021

[CLOSED] APIs to open a folder with file pre-selected brackets-archive/brackets-shell#147

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add "Show in OS" context menu item to open a folder in Finder/Explorer. #2128

Add "Show in OS" context menu item to open a folder in Finder/Explorer. #2128

peterflynn commented Nov 15, 2012

peterflynn Nov 16, 2012

redmunds commented Feb 13, 2013

GarthDB commented Feb 13, 2013

FYI, there's an extension that does this (https://github.com/ujjaval/Open-containing-folder), so maybe we don't need this in core?

peterflynn commented May 14, 2013

gruehle May 14, 2013

gruehle commented May 14, 2013

gruehle May 14, 2013

gruehle commented May 14, 2013

peterflynn commented May 16, 2013

gruehle commented May 17, 2013

lkcampbell commented May 18, 2013

WebsiteDeveloper commented May 18, 2013

lkcampbell commented May 18, 2013

lkcampbell commented May 18, 2013

RaymondLim commented May 18, 2013

WebsiteDeveloper commented May 18, 2013

RaymondLim commented May 18, 2013

RaymondLim commented May 18, 2013

lkcampbell commented May 18, 2013

WebsiteDeveloper commented May 18, 2013

lkcampbell commented May 18, 2013

WebsiteDeveloper commented May 18, 2013

lkcampbell commented May 19, 2013

Add "Show in OS" context menu item to open a folder in Finder/Explorer. #2128

Add "Show in OS" context menu item to open a folder in Finder/Explorer. #2128

Conversation

peterflynn commented Nov 15, 2012

peterflynn Nov 16, 2012

Choose a reason for hiding this comment

redmunds commented Feb 13, 2013

GarthDB commented Feb 13, 2013

FYI, there's an extension that does this (https://github.com/ujjaval/Open-containing-folder), so maybe we don't need this in core?

peterflynn commented May 14, 2013

gruehle May 14, 2013

Choose a reason for hiding this comment

gruehle commented May 14, 2013

gruehle May 14, 2013

Choose a reason for hiding this comment

gruehle commented May 14, 2013

peterflynn commented May 16, 2013

gruehle commented May 17, 2013

lkcampbell commented May 18, 2013

WebsiteDeveloper commented May 18, 2013

lkcampbell commented May 18, 2013

lkcampbell commented May 18, 2013

RaymondLim commented May 18, 2013

WebsiteDeveloper commented May 18, 2013

RaymondLim commented May 18, 2013

RaymondLim commented May 18, 2013

lkcampbell commented May 18, 2013

WebsiteDeveloper commented May 18, 2013

lkcampbell commented May 18, 2013

WebsiteDeveloper commented May 18, 2013

lkcampbell commented May 19, 2013