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

[Feature] Add "$PWD" shell evaluation #1804

Open
1 of 2 tasks
yinzara opened this issue Sep 4, 2020 · 8 comments · May be fixed by #6397
Open
1 of 2 tasks

[Feature] Add "$PWD" shell evaluation #1804

yinzara opened this issue Sep 4, 2020 · 8 comments · May be fixed by #6397
Labels
enhancement New feature or request good first issue Good for newcomers shell This issue is about @yarnpkg/shell

Comments

@yinzara
Copy link

yinzara commented Sep 4, 2020

  • I'd be willing to implement this feature
  • This feature can already be implemented through a plugin (it would require a change to yarnpkg-shell)

Describe the user story

$PWD or ${PWD} is the default Bash environment variable to refer to the current working directory. To allow for cross platform shell evaluation, the yarkpkg-shell should support expanding that environment variable as well.

Describe the solution you'd like

Modify yarnpkg-shell to have a special case variable replacement of "PWD" to the current working directory (i.e. process.cwd()).

Describe the drawbacks of your solution

If a user was using PWD as an environment variable to mean something else, this could break that use case but since the primary case of the Yarn shell is to replicate bash (which reserves that environment variable) the risk is very low.

Describe alternatives you've considered

Using the cross-env package with '$PWD' escaped in single quotes

@yinzara yinzara added the enhancement New feature or request label Sep 4, 2020
@yinzara
Copy link
Author

yinzara commented Sep 4, 2020

I believe this would just require changing:
https://github.com/yarnpkg/berry/blob/master/packages/yarnpkg-shell/sources/index.ts

The evaluateVariable function already supports a "RANDOM" special case. We would just add another for "PWD" to return process.cwd()

@arcanis
Copy link
Member

arcanis commented Sep 5, 2020

While I don't have a super strong feeling against $PWD, I personally don't find it incredibly useful considering that we already have $(pwd), with pwd being a portable builtin.

@paul-soporan
Copy link
Member

I agree that $PWD is not that useful because of the existence of $(pwd) (even though I think we should still add it for better bash compliance), while thinking about this issue $OLDPWD came to my mind which would be useful to have (e.g. IIRC someone needed it for a shared script use case "g:format": "cd $INIT_CWD && prettier --config-path=$OLDPWD", but I didn't remember $OLDPWD existed at that time). What do you think about adding both $PWD and $OLDPWD to the shell?

@arcanis
Copy link
Member

arcanis commented Sep 5, 2020

That sounds reasonable 👍

@arcanis arcanis added the good first issue Good for newcomers label Sep 5, 2020
@yinzara
Copy link
Author

yinzara commented Sep 5, 2020

I'd love to add OLDPWD, however I'm not sure how I'd accomplish that in a non-bash environment? How can I know what the last known working directory was in NodeJS?

@paul-soporan
Copy link
Member

@yinzara The shell has a state which contains the cwd, which is only modified by the cd builtin. Because of this, I think that the best option would be to initialize OLDPWD when the shell starts, and then modify it when cd is called. For PWD it's much simpler, as you only have to return state.cwd.

@arcanis
Copy link
Member

arcanis commented Sep 6, 2020

Technically PWD and OLDPWD aren't magical at all (contrary to RANDOM, for instance, which has a special resolution logic) - it's just that they both get set in the env each time you call the cd builtin. For example:

❯ /mnt/c/Users/nison ❯ PWD=lol

❯ /mnt/c/Users/nison ❯ echo $PWD
lol

❯ /mnt/c/Users/nison ❯ cd

❯ /home/arcanis ❯ echo $PWD
/home/arcanis

@yinzara
Copy link
Author

yinzara commented Sep 6, 2020

I could modify the yarn shell to initialize both the OLDPWD and PWD environment variables to process.cwd() when it starts then modify the environment variables in the "cd" builtin.

If I was within the Yarn shell for the entirety of my execution this wouldn't be an issue, however if one of the main use of yarn shell is to execute scripts from a package.json, I would expect that $OLDPWD would work within a script giving me the working directory of the last directory working directory I was in when I executed the package script.

However given the above implementation, if I evaluated echo $OLDPWD in a package.json script, it would actually be the same value as $PWD as I had never executed the cd buildin.

Thoughts?

@paul-soporan paul-soporan added the shell This issue is about @yarnpkg/shell label Sep 16, 2022
@tthijm tthijm linked a pull request Jul 16, 2024 that will close this issue
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers shell This issue is about @yarnpkg/shell
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants