Community
Participate
Working Groups
"Highlight enclosing brackets" doesn't work well without "Highlight at caret location". Bug 366400 comment 8 point 5 is still broken. More generally: Whenever you move into a pair of brackets from the outside (from left or from right), the crossed bracket is not highlighted again. Before we working on a fix, consider whether "Highlight enclosing brackets" without "Highlight at caret location" makes sense at all from a usage point of view. If you're interested in enclosing brackets, why would you want to lose this information when you get close to a bracket? I don't see a use case for this, and I think the first suboption should be renamed back to "Highlight both brackets". The third option should be a sub-option of the second option, and the third could be enabled by default (but it's still implicitly disabled out of the box because the second option is disabled).
(In reply to comment #0) > Before we working on a fix, consider whether "Highlight enclosing brackets" > without "Highlight at caret location" makes sense at all from a usage point of > view. If you're interested in enclosing brackets, why would you want to lose > this information when you get close to a bracket? I don't know :-) To me not highlighting the bracket at caret location, with or without enclosing brackets enabled, is just wrong. I wanted to chuck this preference and always highlight both brackets... > I don't see a use case for this, and I think the first suboption should be > renamed back to "Highlight both brackets". The third option should be a > sub-option of the second option, and the third could be enabled by default (but > it's still implicitly disabled out of the box because the second option is > disabled). Isn't that a bit complicated? Also, the third option as a sub-option of the second option looks a bit odd...
Not highlighting at the caret location makes sense when you're typing e.g. a closing ')': The box around the typed character can be distracting, but the matching bracket highlight is ambient information that is less intrusive but still interesting. This workflow is currently broken, but bug 372516 will fix that.
(In reply to comment #0) > If you're interested in enclosing brackets, why would you want to lose > this information when you get close to a bracket? (In reply to comment #2) > Not highlighting at the caret location makes sense when you're typing e.g. a > closing ')': The box around the typed character can be distracting. The above two sound a bit contradictory, no? Either you want the bracket at caret location highlighted or you don't. Is the fact that you just typed the bracket or not make that big a difference in the 'distraction' levels? Other possible solution to the problem could be to use a different way of highlighting the brackets, which do not interfere with the caret and then highlight both all the time. Notepad++ just paints the matched brackets themselves in red color. I quite like that look and feel.
(In reply to comment #3) Sounds contradictory, but isn't, since the two statements apply to different settings. The distraction is that the box appears/disappears in situations where that's not interesting information. With enclosing brackets enabled, moving the caret out of a pair of brackets has one interesting state change (from inner brackets to outer brackets) but with the bad setup you'd get two state changes (inner brackets, one bracket hides, outer brackets). When you move into the pair, the late appearance of the box looks like an off-by-one bug. With both options disabled, you don't care about enclosing brackets, but you'd like to see the peer of the bracket at the caret location. The blinking of the peer the confirms that you closed the group you meant to close. Blinking at the caret as well would not give you more information.
We will fix the preference UI for M6, but the bug itself (see comment 0) still needs to be investigated and fixed (can happen during M7).
The current wording on the preference page has a few detail bugs that also make the options hard to understand. Here's a version that removes the bad combination and clarifies the others: [x] &Bracket highlighting o &Matching bracket o Ma&tching bracket and caret location o &Enclosing brackets
Created attachment 212665 [details] fix
(In reply to comment #7) > Created attachment 212665 [details] > fix The patch just tweaks the UI. Regarding the bug in underlying preferences.. - We can simply add a disclaimer to MatchingCharacterPainter.setHighlightCharacterAtCaretLocation(boolean)- "Calling this method has no effect when enclosing brackets are enabled" - And then just always paint both brackets when 'fHighlightEnclosingPeerCharacters' is true. Is this what we are looking for? This can be fixed today itself..
(In reply to comment #8) > Regarding the bug in underlying preferences.. > - We can simply add a disclaimer to > MatchingCharacterPainter.setHighlightCharacterAtCaretLocation(boolean)- > "Calling this method has no effect when enclosing brackets are enabled" > - And then just always paint both brackets when > 'fHighlightEnclosingPeerCharacters' is true. > > Is this what we are looking for? This can be fixed today itself.. Nevermind. Had a chat with Markus and understood that this is not what we want to do.
The patch works but the code has issues: - widgetSelected(...) is called on both buttons when going from on radio to another (see Javadoc for details). Hence you can't just adjust the prefs without checking (or using) the state. - use getSelection() on the fields, that's simpler than getting the widget and casting it to a button - should use getPreferenceStore() and not introduce 'fStore' - typo: checkox - addButton(...) uses wrong variable name (copied code)
Thanks Dani! I pushed the final fix to master http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=310b5a4861b7c93e44dc110dca42def5aace82ab
Verified the new preference UI in I20120314-1800.
Created attachment 212732 [details] additional fix There is a small problem in master (and in I20120314-1800), on clicking 'Restore defaults' the defaults are restored in the underlying preference file but the UI for the reworked options is not updated to reflect the same. The patch fixes the problem.
(In reply to comment #13) > Created attachment 212732 [details] [diff] > additional fix Pushed to master - http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=a1a19e6a24a3bb58c04429a0b9ac4bc88fe7bf9a
Created attachment 212823 [details] patch for platform text Patch for the bug in Platform/Text. Since we already tweaked the preference UI, to reproduce the bug and test the patch, change the line in MatchingCharacterPainter.setHighlightCharacterAtCaretLocation(boolean) to "fHighlightCharacterAtCaretLocation= false;"
(In reply to comment #15) > Created attachment 212823 [details] [diff] > patch for platform text > > Patch for the bug in Platform/Text. > > Since we already tweaked the preference UI, to reproduce the bug and test the > patch, change the line in > MatchingCharacterPainter.setHighlightCharacterAtCaretLocation(boolean) to > "fHighlightCharacterAtCaretLocation= false;" Thanks. Committed to master: http://git.eclipse.org/c/platform/eclipse.platform.text.git/commit/?id=82ab4ae30e3c1a61168a9cf156385d9a3b84c1fb
