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

Remove field_identifier from keyed_element #71

Merged
merged 1 commit into from
Mar 28, 2022

Conversation

adonovan
Copy link
Contributor

@adonovan adonovan commented Mar 25, 2022

This change removes field_identifier from keyed_literal, since in the expressionT{k: v}, k may be:

  • an expression, if T is a map, slice, or array type;
  • a composite literal without an explicit type, if T is an array type;
  • a field identifier, if T is a struct type.

There is no way to distinguish the first two cases without type information. Since expressions include identifiers, it's not necessary to add a special case for field_identifier, and it is incorrect when the identifier is actually a var or const.

Also, factor the grammar using commaSep.

Checklist:

  • All tests pass in CI.
  • There are sufficient tests for the new fix/feature.
  • Grammar rules have not been renamed unless absolutely necessary.
  • The conflicts section hasn't grown too much.
  • The parser size hasn't grown too much (check the value of STATE_COUNT in src/parser.c).

@adonovan adonovan force-pushed the simplify-composite-literal branch from c047d70 to 6c0eb66 Compare March 25, 2022 20:41
@adonovan adonovan changed the title Simplify composite_literal Remove field_identifier from keyed_element Mar 25, 2022
@adonovan adonovan marked this pull request as ready for review March 25, 2022 20:59
@aryx aryx requested a review from maxbrunsfeld March 25, 2022 21:11
@aryx
Copy link
Contributor

aryx commented Mar 25, 2022

The tests need to be updated

@adonovan
Copy link
Contributor Author

The tests need to be updated

Of course. I did update them, and then all the corpus text files got reformatted unnecessarily, and I thought I'd finally found a way to avoid that, but it turns out it was by accidentally removing the tests from the PR. Ugh, I don't know what's going on with me this week.

PTAL.

@adonovan adonovan force-pushed the simplify-composite-literal branch from 6c0eb66 to 5f5c70b Compare March 25, 2022 21:15
@adonovan adonovan requested a review from aryx March 25, 2022 21:16
@aryx aryx merged commit e9ae268 into master Mar 28, 2022
@maxbrunsfeld maxbrunsfeld deleted the simplify-composite-literal branch March 28, 2022 16:25
jimeh added a commit to jimeh/.emacs.d that referenced this pull request Oct 4, 2022
…rties

The field_identifier type was recently removed from tree-sitter-go here:
tree-sitter/tree-sitter-go#71

This means that struct keys are no longer syntax highlighted as
properties, which I dislike. Hence this new query targets the first
element of a key/value pair, and marks the first element as a @Property
if it contains an identifier.

I'm not sure if this undoes the goal of removing the field_identifier,
but for now, it resolves my personal annoyance of struct keys not be
highlighted as properties when creating a new instance of the struct.
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.

3 participants