Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Allow nls before pipe operator #236

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

En3Tho
Copy link
Contributor

@En3Tho En3Tho commented Mar 13, 2024

This "sorta" fixes #182
Better than failing completely but stiill wrong so I need a hint how to do this properly:

1, 2, 3
| Foreach-Object { $_ }

This one should work (does with this pr)

1, 2, 3

| Foreach-Object { $_ }

This one should not work (but still does with this pr). Seems like /n/n leads to An empty pipe element is not allowed.
error on pipe character in vscode/pwsh.

I tried to play with new_line_char instead of nls but no matter what I do it does still parse it as nls. Basically I want to check if there is a thing sorta like nls but should only have exactly 1 line feed (nls allows however many of those). Can you help me with that?

Also, a small qol improvement: using -NoProfile flag when starting pwsh to get module version. Fails on my machine due to some extensions loading first. We want to avoid that by launching pwsh witn no profile

@ForNeVeR ForNeVeR self-assigned this Mar 13, 2024
@ForNeVeR ForNeVeR self-requested a review March 13, 2024 20:55
Copy link
Collaborator

@ForNeVeR ForNeVeR left a comment

Choose a reason for hiding this comment

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

I would ask you to extract the -NoProfile part into a separate PR. No objections against merging that right now.

The newline part is significantly more tricky.

You've correctly outlined the problem: our lexer and tokenizer are specifically designed to not separate a single newline from several newlines.

I am not yet sure of the purpose new_line_char and LF token serve in the parser, but maybe they are broken or just unused, never happen to be materialized during the parsing.

The good thing is that I was able to split this somewhat. Pushed a patch to parser/newlines branch, but I'm not sure it will work well enough (tried a couple of examples and they seem to be parsed well, but, you know, I expect at least a lot of the test data to be totally rewritten after this, and it may be dangerous).

So, I'd suggest you to take a look at the new parser in parser/newlines and maybe see if some of your examples will be parsed more correctly by it. If so, then I guess we'll merge that one as well?

See the commit 8aa6d23.

@En3Tho
Copy link
Contributor Author

En3Tho commented Mar 25, 2024

@ForNeVeR Sorry for the delay. Your fix seems to fix my case. I've mostly did a visual check for now using vscode/current pwsh plugin/your branch. What are the next steps here? Should I close this one and add tests to the one you've mentioned?

@ForNeVeR
Copy link
Collaborator

Alright, I've opened a PR from that branch. This PR will be superseded by that.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Parsing error of newline + pipe operator combination
2 participants