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

Nested block comments #10730

Closed
fricklerhandwerk opened this issue May 17, 2024 · 8 comments · Fixed by #10755
Closed

Nested block comments #10730

fricklerhandwerk opened this issue May 17, 2024 · 8 comments · Fixed by #10755
Assignees
Labels
bug language The Nix expression language; parser, interpreter, primops, evaluation, etc

Comments

@fricklerhandwerk
Copy link
Contributor

fricklerhandwerk commented May 17, 2024

Nested block comments apparently aren't a thing in the Nix language. How sad.

Steps to reproduce

$ nix --version
nix (Nix) 2.22.1
# okay.nix
/*
  okay
*/
1
$ nix-instantiate --eval okay.nix
1
# nope.nix
/*
  /*
    nope
  */
*/
1
$ nix-instantiate --eval nope.nix
error: syntax error, unexpected '*'
       at nope.nix:5:1:
            4|   */
            5| */
             | ^
            6| 1

Trivia

I'm actually surprised how no one seems to have stumbled over this.

Thanks to @hsjobeki's RFC 145 there is even a use case for nested block comments: documenting how to document Nix language libraries within a Nix language library!

@fricklerhandwerk fricklerhandwerk added bug language The Nix expression language; parser, interpreter, primops, evaluation, etc labels May 17, 2024
@FranciscoTGouveia
Copy link

Hi, my name is Francisco and I'm a computer science student that is fascinated by the wonders of Nix. With that being said, I would like to start contributing to the project and think that this could be a good first issue for me to tackle. I've already delved into the codebase an think that the problem might be in the lexical analysis (the Flex file at src/libexpr/lexer.l). I've looked into some closed issues and merged PR's and don't find many assigned issues. Could I be assigned to this issue, or issue assignments are not really a part of the project's culture? Thanks in advance!

@hsjobeki
Copy link
Contributor

hsjobeki commented May 18, 2024

As a reference jsdoc has the same problem. Here is the 10 year old issue:

jsdoc/jsdoc#821

Many languages seem to just not support nested block comments.

still would be nice if nix did.

Workarounds:

  • use html escape sequences (requires some html processing)
  • use single line comments
  • import from external source in doc-comments (see my nixdoc PR for that)

@fricklerhandwerk
Copy link
Contributor Author

@FranciscoTGouveia great to have you! I assigned the issue to you as requested, please add or ping me as a reviewer for PRs.

Note that since we're very conservative with evolving the language, we'll have to make sure that the change is strictly additive and doesn't break existing expressions. We may therefore want to add a test that evaluates past Nixpkgs releases.

@roberth
Copy link
Member

roberth commented May 20, 2024

Concretely, an expression like this may cease to be valid:

# regression.nix
/*
  ignore src/*
*/
1

This instance could be mitigated by requiring whitespace to precede the nested comment.

We can't know the true extent of such issues, because of private code bases.

A simpler solution to the doc comments use case is to recommend that the commented text be unescaped by documentation tooling, according to some rules to be specified. For example, nixdoc could turn *\/ could into */, but a more complete scheme would be desirable.

EDIT: escaped the wrong end

@hsjobeki
Copy link
Contributor

hsjobeki commented May 21, 2024

Another idea:

/**
  comment
  /* nested */
**/
1

Then this would work

# regression.nix
/**
  ignore src/*
**/
1

But this would break:

# regression.nix
/**
  oh no :/
*/
1

@fricklerhandwerk
Copy link
Contributor Author

Apparently I'm ignorant and full of wishful thinking about how computer languages work, but apparently saying a naive thing such as "the longest sequence of characters between /* and */ is a comment" is an instance of greedy matching, which would make parsing a lot less efficient.

@roberth
Copy link
Member

roberth commented May 21, 2024

I don't know where you got that, but the longest sequence would also ignore 2 in /**/2/**/, so I don't think we want that anyway.
If we do the unescaping thing, we can keep the parsing (or lexing) the same as it is today. Unescaping is quite cheap and can be done on demand.

@fricklerhandwerk
Copy link
Contributor Author

@roberth agreed, recommending unescaping is the least invasive and simplest thing to do here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug language The Nix expression language; parser, interpreter, primops, evaluation, etc
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants