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

indentWithTabs ought to preserve C-style multiline comments #1070

Closed
nedtwigg opened this issue Jan 6, 2022 · 2 comments · Fixed by #1072
Closed

indentWithTabs ought to preserve C-style multiline comments #1070

nedtwigg opened this issue Jan 6, 2022 · 2 comments · Fixed by #1072

Comments

@nedtwigg
Copy link
Member

nedtwigg commented Jan 6, 2022

As noted here, our indentWithTabs space has some surprising behavior when used with C-style multiline comments, for example:

/**
 * A simple greeting task.
 */
abstract class GreetingTask extends DefaultTask {}

Gets errored out because Spotless dumps the leading spaces

/**
-·*·A·simple·greeting·task.
-·*/
+*·A·simple·greeting·task.
+*/

A hard limitation is that the indentWithTabs step can't know what language it is indenting. But it is pretty easy to detect that the leading whitespace of a line was terminated by a *, and then note that the indentation had one more space than expected, and just preserve that space.

You would detect that the first non-whitespace character is a * here to set boolean mightBeMultilineComment = true/false.

char c;
while (contentStart < raw.length() && isSpaceOrTab(c = raw.charAt(contentStart))) {
switch (c) {
case ' ':
++numSpaces;
break;
case '\t':
numSpaces += state.numSpacesPerTab;
break;
default:
throw new IllegalArgumentException("Unexpected char " + c);
}
++contentStart;
}

And then do some remainder math here to add an extra space if necessary.

case TAB:
for (int i = 0; i < numSpaces / state.numSpacesPerTab; ++i) {
builder.append('\t');
}
break;

I don't plan to implement this, but I'm happy to merge a PR for this (current test is here).

@nedtwigg
Copy link
Member Author

nedtwigg commented Jan 6, 2022

@sormuras was nice enough to contribute something similar to this back in #82, but I rejected it at that time as a breaking change. The key change in scope that I believe makes this a ### Fixed rather than a **BREAKING CHANGE** is:

  • detects that first non-whitespace characters is *
  • then and only then it will permit a single extra space character to persist

vpavic added a commit to vpavic/spotless that referenced this issue Jan 6, 2022
At present, `indentWithTabs` doesn't handle leading space on multi-line comments and reports errors on properly formatted code.

This commit updates `IndentStep` to add support for detecting multi-line comments and allowing a single leading space for such lines.

Closes diffplug#1070
vpavic added a commit to vpavic/spotless that referenced this issue Jan 6, 2022
At present, `indentWithTabs` doesn't handle leading space on multi-line comments and reports errors on properly formatted code.

This commit updates `IndentStep` to add support for detecting multi-line comments and allowing a single leading space for such lines.

Closes diffplug#1070
@vpavic
Copy link
Contributor

vpavic commented Jan 6, 2022

I opened #1072 to address this.

The proposed solution does this:

  • detects that first non-whitespace characters is *
  • then and only then it will permit a single extra space character to persist

vpavic added a commit to vpavic/spotless that referenced this issue Jan 6, 2022
At present, `indentWithTabs` doesn't handle leading space on multi-line comments and reports errors on properly formatted code.

This commit updates `IndentStep` to add support for detecting multi-line comments and allowing a single leading space for such lines.

Closes diffplug#1070
nedtwigg added a commit to vpavic/spotless that referenced this issue Jan 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants