Skip to content

Conversation

@AdrieanKhisbe
Copy link
Contributor

protect case option with () to prevent parse error

should fix #104

@molovo
Copy link
Member

molovo commented Sep 18, 2017

Thanks for the PR, though I do want to look into the error you received a little more before merging this.

I'm not seeing this error in any versions of ZSH I've tried. The syntax before this change is perfectly valid and should not have raised a parse error, which leads me to believe something else is going on.

Would you mind posting a copy of your .zshrc (with any sensitive information redacted)? I'd like to try and recreate this in a VM if I can.

Note to self: I really must enable OSX builds on Travis again

@AdrieanKhisbe
Copy link
Contributor Author

No problem.
Could I send it to you via private message though?

@molovo
Copy link
Member

molovo commented Sep 18, 2017

Absolutely, my email is [email protected] :)

@AdrieanKhisbe
Copy link
Contributor Author

voilà :)

@molovo
Copy link
Member

molovo commented Sep 18, 2017

Got it, thanks. I'll take a look at this at some point tomorrow

@AdrieanKhisbe
Copy link
Contributor Author

No emergency :)

@molovo
Copy link
Member

molovo commented Oct 1, 2017

I never managed to get to the bottom of this - spent a while looking at your .zshrc but still couldn't recreate the problem. Since this a) is a purely syntactical change, b) fixes the issue, and c) doesn't break the unit tests, I'm going to merge it.

Before I do that, @AdrieanKhisbe could you please rebase your branch on the next branch of Zulu, and then change this PR to target the next branch. Then it can be included in v1.5.0.

@AdrieanKhisbe AdrieanKhisbe force-pushed the fix-parse-error-case-sync branch from 5c2ef47 to aadea31 Compare October 1, 2017 19:36
@AdrieanKhisbe
Copy link
Contributor Author

Strange Strange. Thanks a lot for the time invested !

I just rebased as you requested :)

@AdrieanKhisbe
Copy link
Contributor Author

Have you a target in mind for 1.5.0?

@molovo
Copy link
Member

molovo commented Oct 1, 2017

Yeah, the next branch please. Then, you can start using it as soon as it's merged by changing zulu init in your .zshrc to zulu init --next since 1.5 might not be released soon

@molovo molovo changed the base branch from master to next October 2, 2017 22:43
@molovo
Copy link
Member

molovo commented Oct 2, 2017

Oh, I can change the base myself. Awesome. LGTM. Thanks for the PR @AdrieanKhisbe

@molovo molovo merged commit bde1071 into zulu-zsh:next Oct 2, 2017
@AdrieanKhisbe AdrieanKhisbe deleted the fix-parse-error-case-sync branch October 2, 2017 22:44
@AdrieanKhisbe
Copy link
Contributor Author

I don't know why I thought I couldn't ^^

@molovo molovo mentioned this pull request Oct 2, 2017
7 tasks
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.

2 participants