Don't convert paths to real paths when building text buffers #13545

Merged
merged 5 commits into from Jan 5, 2017

Projects

None yet

2 participants

@maxbrunsfeld
Contributor
maxbrunsfeld commented Jan 4, 2017 edited

Refs #9879
Refs #10856
Refs atom/text-buffer#182
Refs facebook/nuclide#697

/cc @dirk-thomas I have talked to some others on our team, and reconsidered this problem. I feel really bad for the delay in resolving this issue, and all of the work that you've done to propose solutions.

Where we stand right now, we think that the right solution is probably to simply remove the conversion to real paths. There may be some breakage of community packages, but it's hard for me to see how.

Please let me know what you think.

Discussed this with @nathansobo
/cc @noseglid because you maintain atom-build and this relates to noseglid/atom-build#245

@maxbrunsfeld maxbrunsfeld Don't convert paths to real paths when building text buffers
ed5c1c1
@dirk-thomas
Contributor
dirk-thomas commented Jan 4, 2017 edited

No worry, I know how busy it can be. Even when I poke more frequently on the tickets I can emphasize with the amount of tickets you have to deal with.

In atom/atom#9879 the concern was brought up that such a change might be too much to be introduced in Atom 1.x. But I agree that just updating the existing resolvePath as in this patch is definitely the cleaner more future proof approach. I would be very happy if this get merged into an upcoming Atom release. I am sure I can then address the original problem in the build packages to support workspaces which contain symlinks. 👍

@maxbrunsfeld
Contributor
maxbrunsfeld commented Jan 4, 2017 edited

Ok, if you're cool with this and it passes our CI (or can be made to pass with minor adjustments to the bundled package test suites) it'll go out in the Atom 1.14 Beta release in a couple of weeks.

@maxbrunsfeld
Contributor

It's also just one less synchronous file system call 🐎 .

@maxbrunsfeld maxbrunsfeld referenced this pull request in atom/node-pathwatcher Jan 4, 2017
Merged

Don't convert file paths to real paths in Directory.resolve #116

maxbrunsfeld added some commits Jan 4, 2017
@maxbrunsfeld maxbrunsfeld ⬆️ pathwatcher (prerelease)
786be6a
@maxbrunsfeld maxbrunsfeld ⬆️ markdown-preview bc0dcf2
maxbrunsfeld added some commits Jan 5, 2017
@maxbrunsfeld maxbrunsfeld ⬆️ tree-view
f42e279
@maxbrunsfeld maxbrunsfeld ⬆️ pathwatcher
0006b05
@maxbrunsfeld maxbrunsfeld merged commit 5a898b2 into master Jan 5, 2017

0 of 5 checks passed

ci/circleci CircleCI is running your tests
Details
continuous-integration/appveyor/branch Waiting for AppVeyor build to complete
Details
continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/travis-ci/push The Travis CI build is in progress
Details
@maxbrunsfeld maxbrunsfeld deleted the mb-preserve-unresolved-paths branch Jan 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment