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

Add &| and 2>| as pipe operators #21

Merged
merged 8 commits into from
Apr 2, 2024
Merged

Conversation

piechologist
Copy link
Contributor

Currently, both operators result in errors in the syntax tree.

This PR adds two convenience pipe operators (see the fish docs) discussed in #20:

echo Piping &| cat
echo Piping 2>| cat

I changed both grammar.js and grammar.ts to be able to build the parser locally. The added tests pass using tree-sitter 0.22.2.

I added the operators to highlights.scm as well.

Closes #20.

@piechologist
Copy link
Contributor Author

Oops, looks like an old node version in a GH action? I tested with node v21.7.1 on my local machine.

@taekwombo
Copy link
Contributor

taekwombo commented Apr 1, 2024

Node should be fine, you're free to upgrade - it's not a big deal.
Error in pipeline comes from @types/node package.
Adding "moduleResolution": "nodenext" to tsconfig.json fixes errors.

This package could be removed by adding single ts-ignore above module.exports in grammar.ts 😅

@taekwombo
Copy link
Contributor

Looks fine ;) Also please include auto generated files from src directory - so that the source is ready to use with your patch applied.

@piechologist
Copy link
Contributor Author

Adding "moduleResolution": "nodenext" to tsconfig.json fixes errors.

That did the trick. I've neither upgraded node nor added ts-ignore. Didn't want to touch so many things at the same time 😉

I've also incorporated #22 (drop ^).

Then ran tree-sitter generate && tree-sitter build locally with the latest tree-sitter v0.22.2. That's what homebrew gave me. So, those files went into src.

Thanks for being patient 😃.

@ram02z ram02z self-requested a review April 2, 2024 17:43
Copy link
Owner

@ram02z ram02z left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm. thanks for contributions, @piechologist !

@ram02z ram02z merged commit a78aef9 into ram02z:master Apr 2, 2024
1 check passed
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.

Allow &| and 2>| in pipes
3 participants