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

unexpected behavior between source and fish #2643

Closed
macb opened this Issue Dec 29, 2015 · 5 comments

Comments

Projects
None yet
3 participants
@macb

macb commented Dec 29, 2015

Stumbled across some unexpected behavior when using source vs fish.

The contents of test.fish:

set myvar (dirname (status -f))
echo $myvar

While using fish as my SHELL and using source to run the file the myvar variable is set to a relative path to the file being run.

➜ mydir source test.fish
.

However, when using fish to run the file the myvar variable is set to the absolute path to the file being run.

➜ mydir fish test.fish
/private/tmp/mydir

Is this expected? If so is there a workaround to keep the behavior consistent between the two of them? In this case we're expecting the absolute path to the file.

@krader1961

This comment has been minimized.

Show comment
Hide comment
@krader1961

krader1961 Dec 30, 2015

Contributor

Both are technically correct and the documentation is ambiguous. It just says "prints the filename of the currently running script." without stating whether it will always result in an absolute path. Having said that I find the inconsistency surprising and would vote for updating the code and docs to state the path will always be absolute.

Contributor

krader1961 commented Dec 30, 2015

Both are technically correct and the documentation is ambiguous. It just says "prints the filename of the currently running script." without stating whether it will always result in an absolute path. Having said that I find the inconsistency surprising and would vote for updating the code and docs to state the path will always be absolute.

@ridiculousfish

This comment has been minimized.

Show comment
Hide comment
@ridiculousfish

ridiculousfish Dec 30, 2015

Member

It's weird that fish would fully resolve the path when passed a file. This was introduced in 06fd1aa a long time ago (the call to wrealpath in main.c). Note that this means the fish test.fish case also resolves symlinks.

I'm unsure if it's better to show the relative path, absolute path, or fully resolved path (i.e. absolute with symlinks resolved). I think that resolving symlinks is bad, because some scripts are symlinked to each other and switch off their name to decide what to do. But as for absolute vs relative, I'm not sure - are there any legitimate uses for knowing the relative path?

Member

ridiculousfish commented Dec 30, 2015

It's weird that fish would fully resolve the path when passed a file. This was introduced in 06fd1aa a long time ago (the call to wrealpath in main.c). Note that this means the fish test.fish case also resolves symlinks.

I'm unsure if it's better to show the relative path, absolute path, or fully resolved path (i.e. absolute with symlinks resolved). I think that resolving symlinks is bad, because some scripts are symlinked to each other and switch off their name to decide what to do. But as for absolute vs relative, I'm not sure - are there any legitimate uses for knowing the relative path?

@ridiculousfish ridiculousfish added this to the fish-future milestone Dec 30, 2015

@ridiculousfish ridiculousfish self-assigned this Dec 30, 2015

@krader1961

This comment has been minimized.

Show comment
Hide comment
@krader1961

krader1961 Dec 30, 2015

Contributor

Yeah, upon further reflection I agree that defining it to always produce a relative path if one was used in the first place makes more sense than always converting to an absolute path since there is a trivial command for doing the latter. Obviously if the user explicitly specifies an absolute path then that is what they'll get from status -f. It'd be good to update the status man page to clarify this point in addition to backing out that change.

Contributor

krader1961 commented Dec 30, 2015

Yeah, upon further reflection I agree that defining it to always produce a relative path if one was used in the first place makes more sense than always converting to an absolute path since there is a trivial command for doing the latter. Obviously if the user explicitly specifies an absolute path then that is what they'll get from status -f. It'd be good to update the status man page to clarify this point in addition to backing out that change.

@ridiculousfish

This comment has been minimized.

Show comment
Hide comment
@ridiculousfish

ridiculousfish Jan 3, 2016

Member

bc32604 makes the fish ./test.fish case not resolve paths, so it is consistent with source in returning a relative path. You can use realpath to resolve it explicitly.

Thanks for reporting this!

Member

ridiculousfish commented Jan 3, 2016

bc32604 makes the fish ./test.fish case not resolve paths, so it is consistent with source in returning a relative path. You can use realpath to resolve it explicitly.

Thanks for reporting this!

@ridiculousfish ridiculousfish modified the milestones: next-2.x, fish-future Jan 3, 2016

@macb

This comment has been minimized.

Show comment
Hide comment
@macb

macb Jan 4, 2016

Ah, thanks for taking a look! The absolute path without the need of another utility like realpath was pretty handy. Definitely better to be consistent though! Will update accordingly with that in mind.

macb commented Jan 4, 2016

Ah, thanks for taking a look! The absolute path without the need of another utility like realpath was pretty handy. Definitely better to be consistent though! Will update accordingly with that in mind.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment