Search in Path for chrome.exe #218
Conversation
Allows finding Scoop installations, among others.
|
@patrickhulce A |
src/chrome-finder.ts
Outdated
| prefixes.forEach(prefix => suffixes.forEach(suffix => { | ||
| const chromePath = path.join(prefix, suffix); | ||
| if (canAccess(chromePath)) { | ||
| installations.push(chromePath); | ||
| } | ||
| })); | ||
|
|
||
| // Search %PATH% for chrome.exe | ||
| process.env.Path?.split(';')?.forEach(prefix => { |
There was a problem hiding this comment.
- shouldn't this be
PATH? Did it work for you asPath? Or are env vars in windows cased like that? - Seems we could search PATH in mac (darwin in our code) and linux too
- I think the prefixes array about could have this added (
...process.env.Path.split(';')), and this extra loop can be removed
There was a problem hiding this comment.
- I got confused between
PathandPATHbecause of Questionable internal casing of property nameprocess.env.Pathnodejs/node#20605. - I'll take a look at adding that too.
- Replaced the second loop.
There was a problem hiding this comment.
On Linux, which is used, so manual PATH parsing is unneeded.
On Darwin, lsregister is used, which I think means it looks up apps:
The Launch Service database controls the Finder's 'Open With' right click, contextual menu.
There was a problem hiding this comment.
We ought to be able to use which for mac too. Oh, and there is which -a to list all the commands on PATH (in the expected order, evaluating PATH left to right such that the first command printed is what the shell would use). There's the question of which should take precedence here: the lsregister result or the PATH result. I'd suggest the path result (although this is probably rare to have chrome on your PATH, I don't even have that and I have multiple chromes installed)
The windows equivalent is where (which by default prints everything matching the PATH), although I believe way you're doing it in the current PR is equivalent so not worth using it (but! who know, maybe there is some strange relic of windows behavior we couldn't possible know about).
What do you think of using where and which -a directly to avoid manual PATH processing? At the least, it may be simpler. very curious if it could be more correct.
There was a problem hiding this comment.
oh, found a reason to use where over doing it manually: in windows there's a way to use semicolons for individual items in the PATH... https://superuser.com/questions/584870/how-can-i-add-a-folder-containing-a-semicolon-to-the-windows-path . so if we use where we don't have to worry about the particulars of parsing PATH at all, which apparently isn't as trivial as unix (in unix/POSIX it is impossible to escape the semicolon)
Feel free to leave the PR as-is if you like, and I can work on a follow-up. I do appreciate the quick response after over a year of inactivity :)
There was a problem hiding this comment.
I don't think using which for macOS is a good idea for this PR since applications are installed differently (and in different formats) and I don't have a machine to test it out.
I implemented which -a without much trouble.
I implemented using where and it detects the binaries properly, but I ran into another issue with Scoop: the default chrome.exe shim that Scoop makes sets --user-data-dir and due to what I perceive as a Chrome bug, the later flags appended by chrome-launcher do not override it.
|
associated #277 |
|
@adamraine will verify the window bits, then we can merge. Sorry for the delay @magneticflux- |
|
I couldn't verify this, the command |
It's because having chrome in PATH is not a typical setup, but it is a somewhat reasonable thing for an advanced user to configure. Can you add the folder where chrome is installed to the PATH, and then confirm that the code in chrome-finder.js is getting the correct path? |
|
Ah thanks. Looks like this works as expected 👍 |
Allows finding Scoop installations, among others.
Closes #214