8000 Remove 'project' command line flag by maxbrunsfeld · Pull Request #17212 · atom/atom · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content
This repository was archived by the owner on Mar 3, 2023. It is now read-only.

Remove 'project' command line flag #17212

Merged
merged 1 commit into from
Apr 25, 2018
Merged

Remove 'project' command line flag #17212

merged 1 commit into from
Apr 25, 2018

Conversation

maxbrunsfeld
Copy link
Contributor
@maxbrunsfeld maxbrunsfeld commented Apr 25, 2018 •
edited
Loading

Fixes #17178

This (temporarily) removes the command line interface to the feature added in #16845. As far as I can tell, that interface never quite worked correctly. In my testing, the paths specified in the given project file were not opened. I think there are actually multiple causes of this:

  1. Because the project specification was not integrated into the state serialization system, once the state was loaded from indexedDB, the paths would immediately be replaced by different paths.
  2. Project paths are also updated after the call to project.replace when the asynchronous open-paths message is received.
  3. As someone reported on the initial PR, some logic in the command line parsing caused the directory containing the project path itself to be opened.

The actual reason that I discovered this is while investigating #17178. macOS users are getting this error when trying to open files in Atom by double clicking them. I added some code to log out the path when that error occurs, and I'm seeing that sometimes when double clicking a file, Atom is getting invoked with a project flag set to some weird non-existent path like /sn_1_01235345. I have no idea what causes that flag to get set.

/cc @matthewwithanm @philipfweiss It seemed like the command line project flag was not an important feature on your end; you mainly needed the Project.replace API. Is that right?

I think we can put back the project flag at some point, but for now I just need to get a hotfix out for #17178 and don't have time to fully backfill tests for this feature, think through how to integrate the project flag with the rest of the system, and avoid the bug described above.

@maxbrunsfeld
Remove 'project' command line flag
85d745c 
From what I can tell, this flag never worked correctly. Instead of
opening the paths specified in the project file, the directory
containing the project file itself would always be opened.
@maxbrunsfeld maxbrunsfeld force-pushed the mb-remove-project-flags branch from 48a0eae to 85d745c Compare April 25, 2018 14:46
@maxbrunsfeld maxbrunsfeld merged commit d233e20 into master Apr 25, 2018
@maxbrunsfeld maxbrunsfeld deleted the mb-remove-project-flags branch April 25, 2018 16:14
maxbrunsfeld pushed a commit that referenced this pull request Apr 25, 2018
@maxbrunsfeld
Merge pull request #17212 from atom/mb-remove-project-flags
6447c2a 
Remove 'project' command line flag
maxbrunsfeld pushed a commit that referenced this pull request Apr 25, 2018
@maxbrunsfeld
Merge pull request #17212 from atom/mb-remove-project-flags
e383971 
Remove 'project' command line flag
@matthewwithanm
Copy link
Contributor

Wow, this is a bummer. But yeah, atom.project.replace() is the most important thing for us.

@kopischke
Copy link
kopischke commented Apr 26, 2018 •
edited
Loading

Forgive me for jumping in, but I think I can shed some light on what happens here, as I was bitten by the underlying macOS quick in an unrelated project recently. As succinctly put in this SO answer:

Mac OS X assigns a unique process serial number ("PSN") to all apps launched via GUI. It's used for identifying various processes and instances of executables.

which, and this is the kicker, is passed in the form of a -psn_* argument, which in turn Atom interprets as the -p short form of the --project switch and makes it process a spurious path.

Note that, unlike stated in the comments to the answer linked above, this behaviour persists up to at least macOS 10.12 Sierra, which is where I experienced it.

@maxbrunsfeld
Copy link
Contributor Author
maxbrunsfeld commented Apr 26, 2018 •
edited
Loading

Ah, thanks so much for the insight @kopischke! I'm so happy to have that question answered.

So yeah, I think that we should add back the --project / -p flag at some point; it makes sense as a feature to expose, but there are enough issues with it that I think for the current patch release, we can just take it out.

@steveward steveward mentioned this pull request Jun 12, 2018
Closed
@ayame113 ayame113 mentioned this pull request Jun 20, 2021
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Intermittent "Unable to read supplied project specification file" error when opening files
3 participants
@maxbrunsfeld @matthewwithanm @kopischke
0