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

Fix dirs returns false when $dirstack is empty #8211

Merged
merged 1 commit into from Aug 9, 2021

Conversation

andrew-schulman
Copy link
Contributor

Description

In fish 3.3.1, when $dirstack is empty, dirs returns false. This is undesirable, because no error occurred.

It happens because the string join at the end doesn't join anything, so it returns false. The solution is just to add a return 0 at the end of the function.

TODOs:

  • Changes to fish usage are reflected in user documentation/manpages.
  • Tests have been added for regressions fixed
  • User-visible changes noted in CHANGELOG.rst

@kidonng
Copy link
Contributor

kidonng commented Aug 8, 2021

I would say current behavior seems right. There is a slight difference between printing nothing and matching nothing, and adding a return 0 unconditionally will make them indistinguishable.

EDIT: I'm being dumb here, just noticed string join " " is changing status, so my above statement is not true.

@krobelus
Copy link
Member

krobelus commented Aug 8, 2021

There is a slight difference between printing nothing and matching nothing, and adding a return 0 unconditionally will make them indistinguishable.

I don't understand, what's the difference here? I don't think "matching" applies here because we just print the dirstack entries one by one.


I think we should probably change string join here. Returning false on string join ' ' x is weird. Returning false on string join ' ' seems okay still, because in that case the output is empty.

@andrew-schulman
Copy link
Contributor Author

andrew-schulman commented Aug 8, 2021

dirs always prints at least one result, namely the current directory. So right now the return value is different depending on whether it prints only one (false) or more than one (true), which doesn't seem useful. It should just return true.

@faho faho added this to the fish 3.4.0 milestone Aug 9, 2021
@faho faho merged commit afef67b into fish-shell:master Aug 9, 2021
@faho
Copy link
Member

faho commented Aug 9, 2021

Yeah, makes sense. Merged!

@andrew-schulman andrew-schulman deleted the bugfix/dirs-returns-false branch September 9, 2021 13:55
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 12, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants