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

"Editor > Typing > Automatically insert at correct position > Braces" conflicts with typing constructors with single Closure or SAM-type parameter #1108

Open
mauromol opened this issue May 6, 2020 · 5 comments

Comments

@mauromol
Copy link

mauromol commented May 6, 2020

Consider this:

package test68

class GBean {
	
	public GBean(String foo, Closure c) { }
	
	public GBean(Closure c) {}
}

and this:

package test68

class Test68 {

	void foo() {
		def g1 = new GBean('foo', |)
		def g2 = new GBean(|)
	}
}

Invoke code assist at "|".

If the option "Automatically insert at correct position" | "Braces" in Java | Editor | Typing is checked and you try to write a constructor call with a closure as its last parameter, the opening brace is pushed outside the round parenthesis, so that the final result is the creation of an anonymous inner class instead of a constructor call.

#1093 fixed the issue for the g1 case in the above example, by considering the presence of the "comma" the discriminator to avoid such a behaviour.
However in the g2 case the problem persists.

I think the common case here is to call the constructor, while creating an anonymous inner class is a much less common case. Especially in Groovy, especially in Java 8+ days.
It's more natural to me to move the cursor after the round closing parenthesis and then opening the brace whenever I really want to create an inner class, rather than letting the editor decide for me that I do not want to call the constructor, but rather to create an anonymous inner class, and hence moving away the cursor from the point where I really wanted to insert my brace...

@eric-milles eric-milles changed the title "Automatically insert at correct position" | "Braces" option prevents from easily calling constructors with a trailing Closure parameter "Editor > Typing > Automatically insert at correct position > Braces" conflicts with typing constructors with single Closure or SAM-type parameter May 6, 2020
@eric-milles
Copy link
Member

Hard to say which is more common, typing anon. inner class expression or calling constructor with single Closure parameter. The "g2" case above requires type inferencing whereas the "g1" case can be understood with a backwards character scan from the cursor position.

@mauromol
Copy link
Author

mauromol commented May 6, 2020

In any case, I will never be tired to say that if there are two possible choices, the one that mostly respect the developer intention should be preferred. If I want to put a brace there, because it's certainly a valid point, the editor should let me do that, before deciding that the brace should be placed in another point. Unless the other point is the only valid one.

Too many times Greclipse tries to be smarter than me and this indeed hurts productivity :-(

@eric-milles
Copy link
Member

This is behavior that is carried over from the Java Editor. It can disabled by unchecking the preference. You could type a comma or colon before the opening brace. You can use content assist to complete the constructor call including a closure literal. You could assign the closure to a local variable.

I get that you don't want to use workarounds, but "developer's intent" is a very difficult thing to detect. You are fully aware of your intent to type a closure literal. But the editor cannot sense that and provides for the possible intent of others in addition to retaining the behavior it has provided for many years. Fully disabling for constructor calls would take away the assist for an anon. inner class.

@mauromol
Copy link
Author

mauromol commented May 6, 2020

The difference is that in Java it makes no sense to put a brace in that position, so guessing the developer's will to write an anonymous inner class could be useful, while in Groovy the story is different. Why should I be forced to disable an effective Java editor aid (whose description is also so generic that it may control brace insertion in so many other cases) just because of an issue I have while typing Groovy code in a specific case? This is a real issue, not the fact that in Groovy code very very rarely you may want to declare an anonymous inner class. Seriously, how frequent is this? For years Greclipse has played bad with anonymous inner classes because they are so rarely used in Groovy that they were considered lower priority (Groovy has closures), now we are even saying that in an example like the above one it's more likely the user wants to write an anonymous inner class rather than to just call the constructor?
When I talk about the developer intent I mean that in case of ambiguity the developer should be trusted more than any potential assumption of some other thing they may have in mind...

@mauromol
Copy link
Author

mauromol commented May 6, 2020

Also, I don't mean to "fully disable" the feature for any constructor call, just when you have a closure argument there.

If the behaviour it has provided for many years is suboptimal, I see no reason why it should not be improved...

@eric-milles eric-milles added this to the v5.0.0 milestone Mar 30, 2023
@eric-milles eric-milles removed this from the v5.0.0 milestone Jun 27, 2023
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