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

Multiline comments in Zed language #4310

Open
philrz opened this issue Jan 11, 2023 · 2 comments · Fixed by #5175
Open

Multiline comments in Zed language #4310

philrz opened this issue Jan 11, 2023 · 2 comments · Fixed by #5175

Comments

@philrz
Copy link
Contributor

philrz commented Jan 11, 2023

In #4308 review, @mccanne noted:

We should add multiline comments (/* ... */) at some point. I think there was a problem a long time ago with the PEG rules. Just need to go back and fix it.

Indeed, we have these working in ZSON, so it would be nice to have for consistency.

@chrismo
Copy link

chrismo commented Jul 8, 2024

Ah, I was looking for pre-existing issues regarding this ... so it's not expected for this to work:

❯ cat op.zed                      
/* This is a one-liner with multiline syntax */
op no_op(): (
  yield this
)

no_op()

❯ echo '{a:1}' | zq -I op.zed -
zq: error parsing Zed in op.zed at line 2, column 11:
op no_op(): (
      === ^ ===

Multi-line comment works only within ZSON data?

I see the documentation is actually consistent here ... maybe if there could be a "Comments" header to help with the doc search? Here's what I see now:

image

I presume with a proper heading, it'd show up in the results more prominently:
image

And maybe there could be a note, calling out the difference between Zed Language and ZSON in the meantime?

@philrz
Copy link
Contributor Author

philrz commented Jul 9, 2024

Thanks @chrismo! The feedback is appreciated and it's good to know the search tool isn't serving us great here. I've just opened PR #5175 to see if the wider team is on board with adding the section header to see if it'll help.

As for the /* */ multi-line comments, I'll poll the team to see if there's hope of addressing it in the parser ASAP rather than divulging yet another gotcha in the docs that users may not take the time to read. 😄 But if we're going to be stuck living with it for a long time yet, yeah, I'd be open to highlighting that more prominently in the docs too.

@philrz philrz linked a pull request Jul 11, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants