Bug 44642 - New formatter should better handle cascaded message
Summary: New formatter should better handle cascaded message
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.0   Edit
Hardware: PC Windows 2000
: P3 normal (vote)
Target Milestone: 3.0 M7   Edit
Assignee: Olivier Thomann CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 45141 (view as bug list)
Depends on:
Blocks: 49178
  Show dependency tree
 
Reported: 2003-10-10 07:52 EDT by Philipe Mulet CLA
Modified: 2004-02-11 08:03 EST (History)
1 user (show)

See Also:

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Philipe Mulet CLA 2003-10-10 07:52:25 EDT 
Build 20031009

The following expression is leading to unoptimal formatting:

scope.problemReporter().typeMismatchErrorActualTypeExpectedType(
	enclosingInstance,
	enclosingInstanceType,
	inheritedBinding.declaringClass.enclosingType());

it should give more importance to breaking arguments than cascade.
Comment 1 Olivier Thomann CLA 2003-10-10 08:26:38 EDT 
This is the kind of things that need to be tune up.
Comment 2 Philipe Mulet CLA 2003-10-10 09:32:47 EDT 
In particular, it should also deal with cases where multiple message senders 
are using arguments. Likely the outermost should break first.
Comment 3 Olivier Thomann CLA 2003-10-10 09:34:58 EDT 
I should not be difficult to set this.
Comment 4 Olivier Thomann CLA 2003-10-31 08:40:35 EST 
*** Bug 45141 has been marked as a duplicate of this bug. ***
Comment 5 Olivier Thomann CLA 2003-11-26 09:07:04 EST 
Now we have:
scope.problemReporter().typeMismatchErrorActualTypeExpectedType(
        enclosingInstance, enclosingInstanceType,
        inheritedBinding.declaringClass.enclosingType());

For me this is good enough.
Comment 6 Olivier Thomann CLA 2003-11-26 12:09:57 EST 
There is something wrong with my latest changes. The splits are not done where
I'd like.
Comment 7 Olivier Thomann CLA 2003-11-26 12:11:38 EST 
Right now I end up with:
scope
        .problemReporter()
        .typeMismatchErrorActualTypeExpectedType(enclosingInstance,
enclosingInstanceType, inheritedBinding.declaringClass
                .enclosingType());

.enclosingType() is at the end of the previous line.
Ideally I'd like to get .problemReporter().typeMismatchErrorActualTypeExpectedType(

on the same line than "scope".
Need to investigate why it is done this way.
Comment 8 Olivier Thomann CLA 2003-11-26 12:22:31 EST 
Now I have:
scope.problemReporter().typeMismatchErrorActualTypeExpectedType(
        enclosingInstance, enclosingInstanceType,
        inheritedBinding.declaringClass.enclosingType());

It looks much better. Does this look the way you expected it to be?
Comment 9 Olivier Thomann CLA 2003-11-26 12:42:55 EST 
I consider this good enough. If not, then reopen it.
Fixed and released in HEAD.
Regression test added.
Comment 10 Philipe Mulet CLA 2003-11-26 15:17:50 EST 
I think I would use a compact mode for arguments once splitting... is it 
configurable ? And if so, does it work ?

It feels more natural to me to default to:
scope.problemReporter().typeMismatchErrorActualTypeExpectedType(
	enclosingInstance,
	enclosingInstanceType,
	inheritedBinding.declaringClass.enclosingType());
Comment 11 Philipe Mulet CLA 2003-11-26 15:18:39 EST 
reopening
Comment 12 Olivier Thomann CLA 2003-11-26 15:27:50 EST 
The compact mode is what I used. No, this is not configurable for now. This 
specific case is a case where we are formatting a cascading message send. This 
is different from a simple message send with arguments.
If this solution is not good enough, then we might want to revisit the whole 
idea of nested alignment.
Comment 13 Olivier Thomann CLA 2003-11-26 15:41:29 EST 
If I change the alignment of message arguments inside a cascading message send
to M_NEXT_PER_LINE_SPLIT , I get:

scope.problemReporter().typeMismatchErrorActualTypeExpectedType(
        enclosingInstance,
        enclosingInstanceType,
        inheritedBinding.declaringClass.enclosingType());

So this could be an option.
Comment 14 Olivier Thomann CLA 2003-11-26 15:42:22 EST 
My concern is that would then be done for any arguments inside the cascading
message send, not just the last one.
Comment 15 Olivier Thomann CLA 2003-11-26 15:46:55 EST 
For example, if you have this:
scope.problemReporter(enclosingInstance, enclosingInstanceType,
inheritedBinding.declaringClass.enclosingType()).typeMismatchErrorActualTypeExpectedType();

It ends up like this:

scope.problemReporter(
        enclosingInstance,
        enclosingInstanceType,
        inheritedBinding.declaringClass.enclosingType())
        .typeMismatchErrorActualTypeExpectedType();

I am not sure this is what you want.
Comment 16 Philipe Mulet CLA 2003-11-26 17:27:02 EST 
Suggestion below, in case of cascade longer than 1 ?

aaa
  .foo(
     bbb, 
     ccc)
  .bar(
     ddd, 
     eee)
Comment 17 Olivier Thomann CLA 2003-11-26 17:30:54 EST 
This is clearly not following the java conventions. I think it is doable using 
different kind of alignment. I will investigate.
Comment 18 Philipe Mulet CLA 2003-11-26 17:58:13 EST 
We should at least support the standard conventions, my suggestion is somewhat 
deluxe...
Comment 19 Olivier Thomann CLA 2003-11-26 18:03:20 EST 
The result following the standard conventions would be:

scope.problemReporter().typeMismatchErrorActualTypeExpectedType(
        enclosingInstance, enclosingInstanceType,
        inheritedBinding.declaringClass.enclosingType());

Always use a compact split.
Comment 20 Olivier Thomann CLA 2004-02-05 15:55:12 EST 
If I allow the alignment kind to be changed for cascaded method invocations, we
can get this:
scope
	.problemReporter(
		enclosingInstance,
		enclosingInstanceType,
		inheritedBinding.declaringClass.enclosingType())
	.typeMismatchErrorActualTypeExpectedType();

I think this is pretty close to what you wanted.
Closed as fixed.
Comment 21 Frederic Fusier CLA 2004-02-11 08:03:47 EST 
Verified for 3.0-M7 with build I200402102000.