Skip to content
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

Return to original logic for ATOM_DISABLE_SHELLING_OUT_FOR_ENVIRONMENT #831

Merged

Conversation

savetheclocktower
Copy link
Sponsor Contributor

Identify the Bug

In macOS or Linux, with an already-open instance of Pulsar, use your shell to change to an atypical directory, then run pulsar .. When the window opens, open the dev tools, then run process.env.PWD in the shell. The returned value will be /, rather than the directory from which you invoked pulsar.

This might also manifest when Pulsar is launched from scratch; not sure.

Description of the Change

This change seems to be the culprit. (I think it just assigns ATOM_DISABLE_SHELLING_OUT_FOR_ENVIRONMENT rather than exporting it, but I try not to know too much about bash for fear that I might one day be asked to write some.) So I reverted it, or at least the part having to do with ATOM_DISABLE_SHELLING_OUT_FOR_ENVIRONMENT.

Alternate Designs

Nah, better to go with something tried and true.

Possible Drawbacks

Can’t think of any.

Verification Process

This was hard to verify because pulsar is symlinked to your app path (in my case, /Applications/Pulsar.app/Contents/Resources/pulsar.sh) and the script assumes it’ll be run in that context. But you can temporarily define the variables in your shell…

export PULSAR_PATH="/Applications"
export ATOM_APP_NAME="Pulsar.app"

…or the equivalents on Linux. Then you can run pulsar.sh directly from its location in your local copy of the repo. You can then repeat the steps from above and verify that process.env.PWD matches the directory you launched Pulsar from.

Release Notes

  • Fixed an issue that prevented Pulsar from inheriting the directory from which the pulsar binary was run.

@DeeDeeG
Copy link
Member

DeeDeeG commented Dec 13, 2023

Haven't tested this myself yet, but the explanation of "exporting vs simply setting a variable" making a difference in unexpected ways is definitely believable. From experience this has turned out to be the issue a lot of the time.

Copy link
Member

@confused-Techie confused-Techie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great! And review is a no brainer, since comparing to the original diff of this file, we can see things are more just back to how they were.

Thanks for putting this one together, and lets get this one merged!

@savetheclocktower savetheclocktower merged commit f65a4e1 into pulsar-edit:master Dec 15, 2023
98 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants