-
Notifications
You must be signed in to change notification settings - Fork 0
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
[CLOSED] Fix for launching Brackets from command line (Win Only) #340
Comments
Comment by nethip @JeffryBooher @ingorichter Please review with when you guys get some time. |
Comment by nethip Adding @RaymondLim @redmunds |
Comment by le717 No no no no. I object to adding Brackets to the PATH. That is usually reserved for runtime utilities or interpreted programming languages (python, java, node) or critical items needed for system stability (PowerShell, GPU drivers, system32). I don't think we need to add Brackets to the PATH just to get command-line arguments working. I think it was @JeffryBooher who brought it up. There's another way to do it. Let me find it real quick. |
Comment by le717 Found it. adobe/brackets#7482 (comment) (referenced section). I think that should work in the same manner as adding it to the PATH, but with less risk. FWIW, adding applications to the system PATH is generally frowned upon, and when the implementation is flawed the entire PATH can be wrecked (I've had this happen before). I highly advise a followup PR to replace the PATH editing with this registry entry (after confirming it works, which I am pretty sure it does) in this same release (1.3). |
Comment by nethip @le717 The registry entry only takes care of launching Brackets using _Run Command..._ not through command line. To be more technical, any tool that uses Once thing to note here is that we are relying on the MSI that is the standard windows installer that for sure understands how to manage the PATH update efficiently (I have tested this with variety of test cases and the installer does a good job of undoing whatever has been done through the installation process, during uninstallation). It would have been a bigger problem if we were updating the PATH variable programatically. Another option is to create an alias to Brackets, in Windows/System32 folder, which is even more uglier than updating the PATH variable. Considering the no of votes 81 for this trello card (https://trello.com/c/dRNqhd2L/349-s-open-file-folder-from-command-line-on-win-and-linux) I think we should support this feature. |
Comment by le717
I agree this feature should be added, but I don't agree this is how it should be added.
The run command is a command prompt, but only runs one command rather than many. Also, did you compare the .bat script with the MSDN docs on registering an application? I think this method will work. I have Word installed and I can open it from the command-line, and it is not in my PATH nor is there a symlink in System32 (which agreed is ugly). |
Comment by nethip I am open towards making this change if this can be done through a registry entry. I could not find a way of to do this. In fact I would recommend this registry way. Can you share the details of .bat registration in the registry. It would surely help me in doing this change. |
Comment by le717 OK, I apologize. I have always assumed the run command was a mini-command prompt. I just tested it now, and you are right, it only works for Run. I still think it is possible without the PATH... |
Comment by le717 Just so I'm totally clear, adding this to the PATH permits what, again? Opening files/folders in Brackets using open with, or opening a command prompt and typing |
Comment by nethip No worries. Let me know if you if you come across any interesting solitons to fix this. To answer to your last question it does both. We have changed the installer to add open with menu to file and folder context menus. Also Brackets can be launched from command line with file/folder name as argument. |
Comment by le717 I found this a while back. It uses the registry to open files from the context menus. https://gist.github.com/wormeyman/75e3036993136c8b1830 As for the command line, I know of 0 programs that add themselves to the PATH just to open files like that. As I said, it is kinda frowned upon to add a program to the PATH in general unless it is an absolute last resort to do something or it is a purely command line utility (git, python, java, etc). GUI programs like Brackets do not fit into those use cases. In fact, adding to the PATH works around functionality Windows already has. I found that after adding that app registration registry key, I could run Maybe a forum topic should be raised asking about this issue. As I said, I know of zero IDEs that add themselves to the PATH, and it almost sounds to me this is a case of "The user doesn't quite know what they are asking". Also, the 81 votes were probably more towards the context menus than adding to the PATH. |
Comment by peterflynn @abose @nethip Let's discuss this a little more before committing to ship as-is -- given that this is optional or not even a feature in many (most?) other Windows editors |
Comment by peterflynn @le717 It's a popular feature in Mac code editors to be able to launch from the command line -- e.g. |
Comment by ryanstewart Sorry for jumping in late. Just did a bit of research, and like @peterflynn I think there's some confusion/difference between the Windows and Mac experience. On Mac, as Peter says, this is a pretty common feature. TextMate and Sublime both enable it. I assumed Sublime behaved the same way on Windows, but it looks like that's not the case. Googling around it looks like you can do this with Sublime, but it's a very manual process. I think most of the votes in the card referenced are from Mac users who want to be able to use Brackets they way they use TextMate or Sublime. So the big question is whether or not we want to automatically do that for Windows users. I kind of think putting it in the debug menu is enough but maybe we need a more explicit warning. @le717 do you think this would be worth emailing out to the Brackets-Dev group about and getting feedback? |
Comment by peterflynn @ryanstewart Even on Mac it's not that clear cut -- I'm pretty sure TextMate requires you to manually modify your path or add an alias somewhere. Not sure if any editors other than Sublime automatically add themselves. Re votes on the card: until this week I think the title/description was Mac-only, so that is probably a safe assumption about what the voters wanted :-) |
Comment by le717
Yes, I do think this is worth getting feedback from Brackets-Dev. |
Comment by nethip @peterflynn @ryanstewart I am not sure if only MAC users had gone and up voted this considering the fact that there are multiple mentions of this being supported on Windows.
We could do what Peter had suggested on Windows like providing a checkbox for users to opt out of this. |
Comment by MarcelGerber I also think this shouldn't be the default behavior on Windows. As @le717 mentioned, usually only command-line only tools add themselves to the PATH; I've got Sublime Text 3 and Notepad++ installed over here and neither of them added themselves to PATH. |
Comment by nethip @marcelgerber That's a very good catch. We are thinking of making this PATH change optional in the installer, through a check box. Now I am thinking should we even do that. However, we are going to add one checkbox in the installer for adding "open with Brackets" menu to file and explorer menu. |
Comment by le717 @marcelgerber brought up a very good point with node, and such a problem can happen very easily (just install Node and let the installer add it to the PATH, then use this PR to install Brackets. Tada!). As I've stated, adding Brackets to the PATH does not need to happen at all. If people want to load files from the command-line, registering Brackets and using
Yes, please do. That's been a long time complaint about the Windows installer (see the very long discussion and resulting Trello card at adobe/brackets#6073), and making this an (I might suggest default, but we'll see) option will help remedy those complaints. |
Comment by peterflynn @marcelgerber Very good catch! Too bad we haven't yet figured out how to rename the executable back to "Brackets-node.exe," since that would have mitigated the problem... @nethip Seems like we could solve using a script for indirection, similar to what we do on Mac. E.g. add a "brackets.bat" to the build in its own directory ( |
Comment by nethip @peterflynn @marcelgerber @le717 @ryanstewart I have created a new PR for this adobe#514. These are the changes in nutshell
So basically now we have a new folder called [INSTALLDIR]\command which contains the script brackets.bat. This is what is going to be added to the PATH so that anyone when type
I have put in some placeholder string for the checkboxes inside the installer. Feel free to suggest strings for these.
|
Issue by nethip
Wednesday Apr 01, 2015 at 12:54 GMT
Originally opened as adobe#510
The change basically fixes the following problems.
Our command line arguments were never respected as Brackets was expecting a full path and we were just passing the file names
Adding the install location to the PATH variable which will allow users to launch Brackets just by typing "Brackets" instead of full path like "C:\Program Files (86)\Brackets\Brackets.exe".
This is a Win only change.
nethip included the following code: https://github.com/adobe/brackets-shell/pull/510/commits
The text was updated successfully, but these errors were encountered: