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

gitstatus_start_impl: declare os variable. #92

Merged
merged 1 commit into from
Jan 12, 2020
Merged

gitstatus_start_impl: declare os variable. #92

merged 1 commit into from
Jan 12, 2020

Conversation

ryneeverett
Copy link
Contributor

When the os variable isn't declared I get the following error on
initialization:

gitstatus_start_impl:28: os: parameter not set

The error is raised by the following line:

[[ -n $os ]] || { os="$(uname -s)" && [[ -n $os ]] }

@romkatv
Copy link
Owner

romkatv commented Jan 12, 2020

It's declared on the next line. What am I missing?

Could you explain how to reproduce this error? Start by typing zsh -df followed by git clone https://github.com/romkatv/gitstatus.git /tmp/gitstatus. What commands must be typed after this to trigger the error?

When the os variable isn't declared I get the following error on
initialization:

> gitstatus_start_impl:28: os: parameter not set

The error is raised by the following line:

    [[ -n $os ]] || { os="$(uname -s)" && [[ -n $os ]] }
@ryneeverett
Copy link
Contributor Author

It's declared on the next line. What am I missing?

So it is. I don't know zsh very well but there seems to be some condition where declaring and defining multiple variables on one line doesn't work.

@romkatv
Copy link
Owner

romkatv commented Jan 12, 2020

Could you explain how to reproduce this error? Start by typing zsh -df followed by git clone https://github.com/romkatv/gitstatus.git /tmp/gitstatus. What commands must be typed after this to trigger the error?

Could you answer this question?

@ryneeverett
Copy link
Contributor Author

Sorry, I can't reproduce in that way and I'm now pretty sure it's an issue with an overly aggressive patch in my distribution.

@romkatv romkatv reopened this Jan 12, 2020
@romkatv romkatv merged commit b86b0e6 into romkatv:master Jan 12, 2020
@romkatv
Copy link
Owner

romkatv commented Jan 12, 2020

Merged. I believe the PR is a no-op but if it helps you then why not.

ryneeverett added a commit to ryneeverett/nixpkgs that referenced this pull request Jan 12, 2020
This is a followup to NixOS#76744.

The patch is still too aggressive because it captures additional local
variables declared in the same line. It should stop when it hits
whitespace.

See romkatv/gitstatus#92.
@ryneeverett
Copy link
Contributor Author

Well thanks for asking the right questions which pushed me toward the true problem. See NixOS/nixpkgs#77580.

dtzWill pushed a commit to dtzWill/nixpkgs that referenced this pull request Mar 12, 2020
This is a followup to NixOS#76744.

The patch is still too aggressive because it captures additional local
variables declared in the same line. It should stop when it hits
whitespace.

See romkatv/gitstatus#92.

(cherry picked from commit be4efc8)
romkatv added a commit that referenced this pull request Mar 30, 2020
gitstatus_start_impl: declare os variable.
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.

None yet

2 participants