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

[java] not enough indentation when using builder pattern (redux) #337

Open
mtaran-google opened this issue Jun 26, 2021 · 5 comments
Open

Comments

@mtaran-google
Copy link
Contributor

On some detailed inspection, I noticed the patch for #124 didn't actually fix the problem with the initial example:

class Clazz {
  void foo() {
    return new Foo.newBlah(
        new Bar().)
  }
}

pressing enter before the last . should indent +4, but currently indents +0

@marijnh
Copy link
Member

marijnh commented Jun 28, 2021

The problem appears to be that the grammar doesn't understand new Foo.newBlah(...) (it assumes the target for the new operator is a class name). Is that new syntax I'm not aware of? What is it called (so I can look it up)?

@mtaran-google
Copy link
Contributor Author

Ah, my bad!

A more representative example is

class Clazz {
  void foo() {
    return Foo.builder(
        Bar.builder().);
  }
}

That one does work (gives a +4 indent before the last period), but only when that last period is already there. When you're writing new code, that's generally not the case:

class Clazz {
  void foo() {
    return Foo.builder(
        Bar.builder());
  }
}

In this version, when pressing enter before the last close paren the indentation goes +0 where it should go +4.

@marijnh
Copy link
Member

marijnh commented Jun 28, 2021

Right, but at that point there isn't really enough information yet to determine whether the expression will continue, so the +0 is reasonable. Does adding the case where the line starts with a period to electricInput in src/cpp.js, so that the editor will reindent the line when you type a period at the start of a line, produce the effect you are looking for?

@mtaran-google
Copy link
Contributor Author

at that point there isn't really enough information yet to determine whether the expression will continue, so the +0 is reasonable.

I'm not sure I follow you. There's an expression in the outer set of parens, it's had one non-empty line in it (Bar.builder()) and if there's another line break following that part, it's almost certainly going to be more of the expression. The other options I see are

  1. A blank line (allowed per https://google.github.io/styleguide/javaguide.html#s4.6.1-vertical-whitespace)
  2. A comment (followed by no other code, only the closing paren)

Option 1 is theoretically possible, but I wouldn't consider it at all normal (since grouping whitespace is typcially useful between parallel pieces of code). Similarly option 2 would be quite strange since comments are usually either above some code to the right of it on the same line.

Or am I missing something?

@marijnh
Copy link
Member

marijnh commented Jun 29, 2021

The mode acts on a parse of the actual code, and doesn't use whitespace-based heuristics to predict what is likely to be added to that code next. Also, the same indentation logic is used to block-indent stuff, so it doesn't really assume the cursor is on/near the start of the line that's being indented.

Again, would setting things up so that lines are reindented when you type a period at their start (after whitespace) help address this?

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

No branches or pull requests

2 participants