Skip to content

Conversation

xdBronch
Copy link
Contributor

closes ziglang/vscode-zig#447
not sure i love <unnamed> but 🤷
are there any other languages that allow such arbitrary identifiers we might be able to take inspiration from?

Comment on lines +103 to +106
\\Method <unnamed>
\\Method <unnamed>
\\Constant <unnamed>
\\Constant <unnamed>
Copy link
Member

Choose a reason for hiding this comment

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

There is no need for a placeholder like <unnamed>. If the symbol name is not valid then render the name token as is:

Suggested change
\\Method <unnamed>
\\Method <unnamed>
\\Constant <unnamed>
\\Constant <unnamed>
\\Method ""
\\Method " "
\\Constant @""
\\Constant @" "

Copy link
Contributor Author

Choose a reason for hiding this comment

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

would it be ok to just always include the @/""? it'd make this logic a lot simpler

Copy link
Member

Choose a reason for hiding this comment

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

Symbol names shouldn't need to include the @ or "" unless it's needed for some reason. What I suggest isn't as simple as it could be but at least it should hopefully avoid any further issues we may have with how we display document symbol names:

Identifier names should be displayed without @"" unless they need to be escaped. We can assume that the identifier has already been canonicalized with zig fmt so rendering the raw identifier token is fine.

Test names with string literals can be displayed without "" unless one of these cases applies:

  • the string literal is empty
  • the string literal has leading or trailing whitespace
  • the string literal contains escape sequences
    Otherwise it's possible to have two test names that can't be distinguished from each other.

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.

Request textDocument/documentSymbol failed when test name is empty

2 participants