Bug 476248 - "Print Margin Column" functionality is confusing
Summary: "Print Margin Column" functionality is confusing
Status: RESOLVED WONTFIX
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Text (show other bugs)
Version: 4.5.1   Edit
Hardware: All All
: P3 normal with 2 votes (vote)
Target Milestone: ---   Edit
Assignee: Stefan Xenos CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 477458 (view as bug list)
Depends on:
Blocks:
 
Reported: 2015-08-31 13:04 EDT by Zorzella Mising name CLA
Modified: 2017-01-09 07:42 EST (History)
7 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Zorzella Mising name CLA 2015-08-31 13:04:18 EDT
Eclipse 4.5 changed the semantics of the "Print Margin Column" in a problematic manner, as per the bug:

https://bugs.eclipse.org/bugs/show_bug.cgi?id=306646

The current UI is:

****************
[ ] Show print margin
    Print margin column [____]
****************

Problem #1: it violates the Principle of Least Astonishment. You can enable/disable the margin correctly from here, but, since the margin is actually most of the time determined by the formatter's "Maximum line width", changing this number here will have no visible impact, and the feature will seem broken.

Problem #2: it removed a functionality that existed before: the ability to set your print margin independently of your formatter's "Maximum line width".

My proposal is to change the UI to:

*******************
[ ] Show print margin
    Default print margin column [____]
[ ] Override default print margin with per-language formatter's "Maximum line width"

*******************

This would make it clear what the value actually does, and, at the same time, allow for a fixed margin, as it existed before.
Comment 1 Dani Megert CLA 2015-08-31 13:10:18 EDT
(In reply to "Z" Zorzella from comment #0)

This looks like overkill to me.
Comment 2 Sergey Prigogin CLA 2015-08-31 13:21:13 EDT
(In reply to Dani Megert from comment #1)

Agree with Dani.
Comment 3 Stefan Xenos CLA 2015-08-31 14:10:31 EDT
"Z" Zorzella sent me some additional info offline, which I'll try to paraphrase here.

I asked him to provide a use-case for having a displayed margin which is different from the margin used by the formatter.

His use-case seemed reasonable: he wanted to minimize the changes in line break positions under trivial refactorings. So rather than having a single hard wrapping point (at 100), he uses a range of acceptable wrapping points (between 80 and 100), such that if he renames a symbol name doing so will often not introduce additional line breaks.

As part of his workflow, he was using the margin to display the minimum (soft) wrap position rather than the maximum one.

I'll call this the "line wrap range use case".

If we wanted to support the line wrap range use-case, we could do it as follows:

- Add formatter support for a range of wrap positions, where the formatter would accept a minimum and maximum wrap position. It would try to preserve any existing line break that fell within that range.

- Add editor support for multiple margins, so that we can display both the minimum and maximum wrap position.

"Z" Zorzella, could you comment on whether you think this would address your use-case?


However, he also brings up a separate (legitimate) point that the labeling on the old preference may be misleading. This is the same thing Dani was concerned about in the original bug, and we should probably address it.

So, my feeling: I don't think we should add support for setting the margin independently of the formatter. If we want to support line wrap ranges, let's do a good job of that.

I'd suggest we use this bug to discuss the labeling of the preference. Let's open a separate bug to discuss supporting line wrap ranges.
Comment 4 Zorzella Mising name CLA 2015-08-31 14:44:40 EDT
Stefan,

thanks for the comments. Having two rulers would certainly be icing on the cake, though I'm not entire sure of the functional semantic that you'd impart to the "min" margin. And if it's just a visual thing, one could say it feels a bit funny  that this would be part of the formatter settings, since each of all the rest of the settings -- to my knowledge -- has an actual formatting function.

You also correctly restated my point that the Preference, as currently worded is at best misleading. If you were to only fix the wording (problem #1), it would be something along the lines of:

********************
  Default print margin column [____] (overridden by per-language formatter's "Maximum line width")
********************

Now, I'm not exactly holding my breath for the "double margin" feature, and thus I would like to re-advocate for what looks to me like a very simple change: support a fixed margin, if such is desired. The very existence of the new checkbox I propose (i.e. even if a person leaves it unchecked) would make the pref that much more understandable.
Comment 5 Stefan Xenos CLA 2015-08-31 14:49:55 EDT
> I'm not entire sure of the functional semantic that you'd impart to 
> the "min" margin.

Let me explain: if you were to reformat the file, any line break which is already between the "min" and "max" margin would not be re-wrapped, even if another word from the following line would fit in that space.

Basically, it would be formatter support for the convention you appear to be following by hand. You could then turn on the formatter as a post-save action and it wouldn't modify any line breaks in your file which fall into the expected range.
Comment 6 Stefan Xenos CLA 2015-08-31 20:26:58 EDT
Dani, what do you think of "Z"'s proposed wording:

Default print margin column [____] (overridden by per-language formatter's "Maximum line width")
Comment 7 Markus Keller CLA 2015-09-16 13:39:24 EDT
*** Bug 477458 has been marked as a duplicate of this bug. ***
Comment 8 Markus Keller CLA 2015-09-16 14:52:20 EDT
(In reply to Stefan Xenos from comment #6)
> Default print margin column [____] (overridden by per-language formatter's
> "Maximum line width")

The problem with your proposed wording is that language-specific editors don't have to override this setting. The Text Editors preference page mentions such problems in a general way on top: "Some editors may not honor all of these settings". Unfortunately, we can't be more specific, since editors are free to override whatever they want. E.g. the Java editor also takes the tab width settings from the formatter.

I think http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=6d5249a7460d4eadb7933705626efb5fbb168d42 should be reverted.

Two possible solutions that would avoid the confusions:

 (1) Allow setting multiple column values and render multiple margins

 (2) Add a separate MarginPainter in the JavaEditor, controllable with a checkbox on the Java > Editor preference page:

     [] Show formatter line wrap margin

Then users can enable one or both margins at will.
Comment 9 Stefan Xenos CLA 2015-09-16 14:58:29 EDT
> I think http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=6d5249a7460d4eadb7933705626efb5fbb168d42 should be reverted.

I strongly disagree.

I've never received so much email from people thanking me for a bugfix than for that one. Unfortunately, they emailed me directly rather than commenting on the bug -- but I can say with certainty that a lot of users really, really liked it.

That's just the people who noticed the change in behavior, took the time to find the bug, and compose an email to the patch author -- and you've got to really really like a fix to go to all that trouble.

Although I agree the current wording is a minor inconvenience that should be addressed, we absolutely should not revert such an important fix.
Comment 10 Sergey Prigogin CLA 2015-09-16 15:00:17 EDT
(In reply to Stefan Xenos from comment #9)

Agree with Stefan.
Comment 11 Stefan Xenos CLA 2015-09-16 15:07:33 EDT
> Two possible solutions that would avoid the confusions:

While those are reasonable enhancements, I don't think they go to the heart of the problem.

In the case of bug 477458, the user was just confused by the change in behavior. There was no indication the new behavior was problematic for the user, and new wording would have resolved the confusion.

In the case of this bug, "Z" Zorzella is using the print margin to support a workflow whereby he doesn't reformat line breaks that fall within a range of acceptable values.

If we intend to support this workflow, it should be supported by the formatter in addition to having a second vertical margin. If we don't intend to support this workflow, there's nothing to do.
Comment 12 Matt Hoover CLA 2015-09-16 15:12:12 EDT
I am one of the multitude of users thankful for Stefan's fix. The bug he fixed had been a major thorn in my side for years. Please don't revert the fix!

If the functionality is confusing, the logical solution is to improve the UI to remove confusion, not to remove the functionality. The functionality is awesome! :-) Thanks for reading.
Comment 13 Stefan Xenos CLA 2015-09-16 15:15:58 EDT
> "Some editors may not honor all of these settings"

I see no such message anywhere on the page which contains the "Print margin column", but I believe such a message would help resolve the user confusion.
Comment 14 Mihai Nita CLA 2015-09-16 18:09:45 EDT
My 2 cents, if it gets to "voting": I love the new functionality.

I wanted something like this for a long time, to the point where I was considering to write a plugin showing different vertical lines depending on file type.
And at some point I have spend one week-end tinkering with the Eclipse code trying to implement something directly there (but laziness won)

I understand that new behavior can be puzzling.
But this is not the setting that is overwritten by formatters.
Think "Insert spaces for tabs" or "tab size"

Idea for an alternative design (that does not add another checkbox)

[ ] Show print margin
    Default print margin column [____][V]

Using a combo-box:
    Default print margin column [_________________42][V]
                                [                120]
                                [                100]
                                [                 80]
                                [Formatter specified]

The combo-box would still allow a custom value, if one wants.

But I am OK if we keep it and don't touch the UI at all.
Yes, it will override the default. It is not the first setting to do that.


Mihai
Comment 15 Dani Megert CLA 2015-10-26 11:34:51 EDT
(In reply to Dani Megert from comment #1)
> (In reply to "Z" Zorzella from comment #0)
> 
> This looks like overkill to me.

I stay with this decision. If someone wants to provide a solution along the lines of comment 8 (2) then that's fine with me (separate bug please).
Comment 16 John McCabe CLA 2017-01-09 07:42:15 EST
(In reply to Matt Hoover from comment #12)
> I am one of the multitude of users thankful for Stefan's fix. The bug he
> fixed had been a major thorn in my side for years. Please don't revert the
> fix!

It wasn't a bug.