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

Support comments in mlr -s files #1343

Closed
johnkerl opened this issue Jul 31, 2023 · 8 comments · Fixed by #1359
Closed

Support comments in mlr -s files #1343

johnkerl opened this issue Jul 31, 2023 · 8 comments · Fixed by #1359
Assignees

Comments

@johnkerl
Copy link
Owner

johnkerl commented Jul 31, 2023

Originally posted by @janxkoci in #1154 (comment)

@janxkoci
Copy link

janxkoci commented Aug 1, 2023

Oh, thanks! 😀 I suggest rewriting the title to something like "allow comments in regular miller scripts (loaded with mlr -s)" or similar..

@johnkerl
Copy link
Owner Author

johnkerl commented Aug 1, 2023

Oops I thought I did -- didn't push 'save' or something -- thank you!

@johnkerl johnkerl changed the title BTW one aspect of comments in miller scripts still trips me up: comments are supported in DSL (e.g. in scripts loaded with mlr put -f) but not in "normal" miller scripts (i.e. loaded with mlr -s). In regular miller scripts only the first line can have a # to allow shebangs, but using the # anywhere else leads to an error. 😕 Support comments in mlr -s files Aug 1, 2023
@johnkerl johnkerl self-assigned this Aug 19, 2023
@johnkerl
Copy link
Owner Author

@janxkoci here's the catch:

https://github.com/johnkerl/miller/blob/6.8.0/internal/pkg/climain/mlrcli_shebang.go#L62-L64

Namely, if the script file is like

#!/usr/bin/env mlr -s
--c2p
filter '${#quantity} != 20'
then count-distinct -f shape
then fraction -f count

-- which is admittedly a corner-case -- then a naïvely coded replace of #.* to end of lines with "" will not do the right thing.

I think the best option (most often correct) will be for mlr -s to by default replace #.* to end of line with "" unconditionally, and with (alas, yet another Miller command-line flag) let people disable that for rarer situations such as the above.

@janxkoci
Copy link

Wait, doesn't the parser treat code in quotes differently? Or in the parentheses for names with special characters - I thought special treatment was the whole point of those 😳

@johnkerl johnkerl reopened this Aug 19, 2023
@johnkerl
Copy link
Owner Author

johnkerl commented Aug 19, 2023

Sorry @janxkoci I think the auto-close of this issue was from PR #1359 being merged -- re-opening!

@johnkerl
Copy link
Owner Author

@janxkoci yes the DSL parser definitely treats code in quotes differently. But the comment-stripping here has to be before the DSL parser is invoked. It's a big of a chicken-and-egg situationj ...

@johnkerl
Copy link
Owner Author

Handling these two # differently:

#!/usr/bin/env mlr -s
--c2p
filter '${#quantity} != 20'
then count-distinct -f shape # comment here
then fraction -f count

would (if I understand correctly) require re-implementing a shell parser in Miller ...

@janxkoci
Copy link

No worries, and you can even close it, if it's implemented. I mostly don't have such problem with field names anyway, or I can just re-label them. I was just surprised that the parser works like this, that's all.. 🤷

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 a pull request may close this issue.

2 participants