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

Add textobjects and indents to c and cpp #1293

Merged
merged 1 commit into from
Jan 4, 2022

Conversation

Flakebi
Copy link
Contributor

@Flakebi Flakebi commented Dec 18, 2021

Somehow, indentation inside conditions doesn't work, i.e.

  while (1 +<hit enter>)

leads to

  while (1 +
  )

but the expected result should be

  while (1 +
    )

But it is a lot better than before.

@Flakebi
Copy link
Contributor Author

Flakebi commented Dec 18, 2021

Also found another problem

  for (;;) {<hit enter>
  }

works correctly, but

  for (;;
    ) {<hit enter>
  }

leads to

  for (;;
    ) {
      <cursor here with one indentation too much
  }

I guess this is an issue outside the textobjects.scm because the tree-sitter-scopes are equivalent for both cases.

@archseer
Copy link
Member

archseer commented Jan 3, 2022

I think some of these were fixed after #1341

Can you resolve the book/ conflict?

@Triton171
Copy link
Contributor

The issues you found are expected behavior, it's just that the current indentation system is (as far as I can see) not powerful enough to correctly indent C/C++ code in all cases.

In the first example, the while_statement is contained in the indent list, so everything after the first line of it will be indented. But ) is in the dedent list which nullifies this indentation. Because of how indentation currently works, indent does not apply to the first line of the node but dedent does.

In the second example, the issue is that both for_statement and compound_statement are in the indent list. As long as they start in the same line, we will only add level of indentation, but when they start on different lines, they both add 1 to the indent level. In total, the body of the for loop is indented 2 levels more than the surrounding code.
Similiarly, the following code would also be indented incorrectly (although I haven't tested it):

for (;;)
   {
      printf("Hello World");
   }

Maybe there is a better way to structure the indent system but I haven't been able to think of one. The main limitation right now is that the indent queries can only specify node types without any additional context. In grammars that reuse a lot of nodes this makes writing indent queries quite hard.

For now, maybe you could try removing the if, while and for statement nodes from the indent list and the ) from the dedent list. This would at least remove that double indentation issue which seems like it could be really annoying. It would break the indentation of

for (;;)
printf("Hello World");

but as far as I know leaving out the braces is considered bad style anyways.

Indentation of single line statements doesn't work, i.e.

  for (;;)<hit enter>
leads to
  for(;;)
  <cursor here>

Only blocks with curly braces are indented.
@Flakebi
Copy link
Contributor Author

Flakebi commented Jan 3, 2022

Thanks for the explanation @Triton171, it makes a lot of sense now.
I removed the for, if, etc. statements as you suggested and also ) from the outdent list (I think a closing ) is usually kept on the same line as e.g. a condition and not on an extra line).
Also added a class textobject for c++ that I missed before.

It seems to work well now.

@archseer archseer merged commit 5b1a628 into helix-editor:master Jan 4, 2022
@archseer
Copy link
Member

archseer commented Jan 4, 2022

@Triton171 I've seen https://github.com/emacsmirror/tree-sitter-indent/blob/5f80e89b7da2074ea7f083b769448eb7026865dc/tree-sitter-indent.el#L83-L104 that uses more scopes to distinguish between different behaviors but I found it harder to write. Maybe we could use sub-scopes so that the behavior could be opt-in? indent.body, indent.all etc

@Flakebi Flakebi deleted the cpp-textobjs branch January 4, 2022 09:07
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