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

Fix Go select indenting #5713

Merged
merged 4 commits into from
Feb 8, 2023
Merged

Fix Go select indenting #5713

merged 4 commits into from
Feb 8, 2023

Conversation

gavincrawford
Copy link
Contributor

Resolves #5610
Select statements previously were incorrectly indented.
Before this PR:
https://asciinema.org/a/VnOgUThOMTeK3gP2nRr5YQmTt
After this PR:
https://asciinema.org/a/pPAl4ghHjTp6TCiIy4yLIeyUf

@the-mikedavis the-mikedavis added S-waiting-on-review Status: Awaiting review from a maintainer. A-language-support Area: Support for programming/text languages labels Jan 28, 2023
@archseer
Copy link
Member

Hmm I think this is still incorrect: we do want to indent the } and code inside each clause, but the toplevel clauses should be same level as the select:

		select {
		case c <- x:
			x, y = y, x+y
		case <-quit:
			fmt.Println("quit")
			return
		}

\cc @Triton171 what would the correct indentation class be here?

@Triton171
Copy link
Contributor

The problem in #5610 is caused by the @outdent query for {. Apart from that, the current queries look fine (in particular select_statement nodes should not be indented, only the communication_case inside). The best way to fix it is to add an exception to the @outdent rule, for example by replacing it with this:

(_
  "}" @outdent
  (#not-kind-eq? @parent "select_statement")) @parent

@gavincrawford
Copy link
Contributor Author

@Triton171 This solves the issue using the concept you mentioned. However, this also incorrectly indents case expressions, and the following blocks of code. Any ideas?

@Triton171
Copy link
Contributor

Triton171 commented Jan 30, 2023

You have an error in your queries (which is also why the pipeline is failing):

Error: "Failed to parse indents.scm queries for go: Query error at 28:32. Invalid capture name parent"

Just add the @parent at the end of the outdent query for }, otherwise the #not-kind-eq? predicate doesn't know which node @parent is supposed to be.

@pascalkuthe pascalkuthe added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from a maintainer. labels Jan 30, 2023
@gavincrawford
Copy link
Contributor Author

The problem in #5610 is caused by the @outdent query for {. Apart from that, the current queries look fine (in particular select_statement nodes should not be indented, only the communication_case inside). The best way to fix it is to add an exception to the @outdent rule, for example by replacing it with this:

(_
  "}" @outdent
  (#not-kind-eq? @parent "select_statement")) @parent

I tried this solution, and ran into the same errors as with the previous one. I'm not sure what you're really suggesting here.

@Triton171
Copy link
Contributor

Triton171 commented Feb 6, 2023

My mistake, I didn't know that tree-sitter requires captures to occur in the query before they are used in a predicate (so contrary to what is suggested in the documentation, the position of predicates inside a pattern actually does matter). Moving the predicate to the end fixes the error. The following indents.scm works for me:

[
  (import_declaration)
  (const_declaration)
  (type_declaration)
  (type_spec)
  (func_literal)
  (literal_value)
  (literal_element)
  (keyed_element)
  (expression_case)
  (default_case)
  (type_case)
  (communication_case)
  (argument_list)
  (field_declaration_list)
  (block)
  (type_switch_statement)
  (expression_switch_statement)
  (var_declaration)
] @indent

[
  "]"
  ")"
] @outdent

((_ "}" @outdent) @outer
  (#not-kind-eq? @outer "select_statement"))

(communication_case) @extend

The @extend query at the end ensures that the whitespace directly after a communication_case is also considered to be part of the case. I also removed the @outdent query for "case" since this produces incorrect indentation.

The only issue I noticed is that after having writing select {}, pressing Enter inside the curly braces creates an unnecessary indent for the new line. This is due to the auto-pairing code (essentially, pressing Enter between auto-pairs always creates a blank line with an additional level of indentation) and I don't think there's currently a way to prevent that for this specific case.

@gavincrawford
Copy link
Contributor Author

That solution works perfectly on my end. No errors, and the subtree looks correct to me.
@Triton171 thanks so much for the help!

@the-mikedavis the-mikedavis added S-waiting-on-review Status: Awaiting review from a maintainer. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 7, 2023
@archseer archseer merged commit 00ecc55 into helix-editor:master Feb 8, 2023
@gavincrawford gavincrawford deleted the go-select-indents branch February 8, 2023 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-language-support Area: Support for programming/text languages S-waiting-on-review Status: Awaiting review from a maintainer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

golang incorrect indentation after select keyword
5 participants