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

Contents of $argv for source'd script (AKA ".") are weird #139

Closed
lrm29 opened this Issue Jun 16, 2012 · 6 comments

Comments

Projects
None yet
5 participants
@lrm29
Contributor

lrm29 commented Jun 16, 2012

Hello,

When using . with no arguments, $argv contains the name of the file being sourced. If any arguments are supplied, then the name of the file being sourced is not in $argv. e.g.;

. FILENAME -> $argv = FILENAME
. FILENAME arg1 -> $argv = arg1
. FILENAME arg1 arg2 -> $argv = arg1 arg2

Is this the desired default behaviour? I'm not sure where to find the . function in the git repository.

Thanks!

@ridiculousfish

This comment has been minimized.

Show comment
Hide comment
@ridiculousfish

ridiculousfish Jun 16, 2012

Member

. is the builtin_source function.

Member

ridiculousfish commented Jun 16, 2012

. is the builtin_source function.

@lrm29

This comment has been minimized.

Show comment
Hide comment
@lrm29

lrm29 Jun 16, 2012

Contributor

Thanks, on line 2853 I changed it to either:

parse_util_set_argv( argv+1, wcstring_list_t());

to include the filename that will be sourced or to:

parse_util_set_argv( argv+2, wcstring_list_t());

if not.

Contributor

lrm29 commented Jun 16, 2012

Thanks, on line 2853 I changed it to either:

parse_util_set_argv( argv+1, wcstring_list_t());

to include the filename that will be sourced or to:

parse_util_set_argv( argv+2, wcstring_list_t());

if not.

@faho faho changed the title from . (dot) -> contents of $argv to Contents of $argv for source'd script (AKA ".") are weird Sep 15, 2015

@faho

This comment has been minimized.

Show comment
Hide comment
@faho

faho Sep 15, 2015

Member

I re-titled this to try to make it a bit more discoverable.

Member

faho commented Sep 15, 2015

I re-titled this to try to make it a bit more discoverable.

@krader1961 krader1961 added the bug label Apr 6, 2016

@krader1961 krader1961 self-assigned this Apr 6, 2016

@krader1961

This comment has been minimized.

Show comment
Hide comment
@krader1961

krader1961 Apr 6, 2016

Contributor

This behavior is wrong. I've confirmed the current git head fish still behaves this way. Also, sh, bash, and zsh do not behave like fish. If you don't pass any args to the sourced script then their equivalent of $argv is empty. Fixing this will break anything depending on the bug but I think this is a case where breaking backward compatibility is worth it.

Contributor

krader1961 commented Apr 6, 2016

This behavior is wrong. I've confirmed the current git head fish still behaves this way. Also, sh, bash, and zsh do not behave like fish. If you don't pass any args to the sourced script then their equivalent of $argv is empty. Fixing this will break anything depending on the bug but I think this is a case where breaking backward compatibility is worth it.

@lrm29

This comment has been minimized.

Show comment
Hide comment
@lrm29

lrm29 Apr 6, 2016

Contributor

How would you advertise the change? Would it be a release note? Would it be possible to warn for a release if $argv is accessed when no arguments are passed?

Contributor

lrm29 commented Apr 6, 2016

How would you advertise the change? Would it be a release note? Would it be possible to warn for a release if $argv is accessed when no arguments are passed?

@krader1961

This comment has been minimized.

Show comment
Hide comment
@krader1961

krader1961 Apr 6, 2016

Contributor

How would you advertise the change? Would it be a release note?

Yes

Would it be possible to warn for a release if $argv is accessed when no arguments are passed?

Yes, it's just a "small matter" of writing a bunch of code then tracking the change to remove it and implement the correct behavior. And merely emitting a warning can itself cause problems. I think we just need to bite the bullet. It's a shame no one fixed this four years ago when you brought it to our attention.

Contributor

krader1961 commented Apr 6, 2016

How would you advertise the change? Would it be a release note?

Yes

Would it be possible to warn for a release if $argv is accessed when no arguments are passed?

Yes, it's just a "small matter" of writing a bunch of code then tracking the change to remove it and implement the correct behavior. And merely emitting a warning can itself cause problems. I think we just need to bite the bullet. It's a shame no one fixed this four years ago when you brought it to our attention.

@krader1961 krader1961 modified the milestones: 2.3.0, fish-future Apr 6, 2016

@krader1961 krader1961 closed this in 02f18ca Apr 6, 2016

@floam floam modified the milestones: fish 2.3.1, fish 2.3.0 Aug 2, 2016

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