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

Use -print-last-dir option for lfcd.cmd #1444

Merged
merged 3 commits into from
Oct 22, 2023
Merged

Use -print-last-dir option for lfcd.cmd #1444

merged 3 commits into from
Oct 22, 2023

Conversation

atahrijouti
Copy link
Contributor

The current script was existing the console itself rather than just exiting lfcd.cmd.
explicit exit code makes sure we're only exiting lfcd.cmd

Copy link
Collaborator

@joelim-work joelim-work left a comment

Choose a reason for hiding this comment

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

Thanks for working on the lfcd.cmd script.

Just out of curiosity, shouldn't you be using exit /b if you want to prevent the script from exiting the console? I tried exit 0 just by itself and it doesn't seem to change anything.

Also, we have been trying to remove the use of temp files ever since the -print-last-dir option was introduced in r31. I was wondering if you would be able to test the following approach instead to see if it works for you:

@echo off
rem Change working dir in cmd.exe to last dir in lf on exit.
rem
rem You need to put this file to a folder in %PATH% variable.

for /f "usebackq tokens=*" %%d in (`lf -print-last-dir %*`) do cd %%d

@atahrijouti
Copy link
Contributor Author

@joelim-work works flawlessly 🎉. Updated the PR with your code (if you're interested in merging it) otherwise I would leave it to you, if you intend to have a backwards compatible approach or a separate lfcd for r31

Just out of curiosity, shouldn't you be using exit /b if you want to prevent the script from exiting the console? I tried exit 0 just by itself and it doesn't seem to change anything.

In my first attempt I used /b, but upon further reading of the docs, I think I understood that it had more semantic meaning and is used in tandem with different types of exit codes. (it was late at night though, so I might have over thought it)

Copy link
Collaborator

@joelim-work joelim-work left a comment

Choose a reason for hiding this comment

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

if you intend to have a backwards compatible approach or a separate lfcd for r31

In general we only support the latest version. The old version of the script will always be stored in the Git history for users that can't upgrade or don't want to for some reason.

Anyway I will approve this PR, but I will leave it open in case @gokcehan or anyone else wants to review it. I have also updated the PR title.

@joelim-work joelim-work changed the title Exit lfcd.cmd not cmd.exe Use -print-last-dir option for lfcd.cmd Sep 27, 2023
@gokcehan
Copy link
Owner

@atahrijouti @joelim-work Looks good, thanks for the patch.

I expect older versions to use the lfcd scripts from the corresponding .zip and .tar.gz files from the releases sections, so it should be ok to make a backwards incompatible change here.

@gokcehan gokcehan merged commit 1a3bcb2 into gokcehan:master Oct 22, 2023
4 checks passed
@atahrijouti atahrijouti deleted the lfcd-batch-exit-lf branch October 22, 2023 19:35
@gokcehan gokcehan mentioned this pull request Mar 31, 2024
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.

3 participants