Bug 22712 - professional editor features: show invisible chars
Summary: professional editor features: show invisible chars
Status: RESOLVED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: Text (show other bugs)
Version: 2.0   Edit
Hardware: PC Windows 2000
: P3 enhancement with 17 votes (vote)
Target Milestone: 3.3 M4   Edit
Assignee: Platform-Text-Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords: contributed, investigate
: 25428 31900 33829 41247 44708 55105 106038 107076 107936 110091 118079 152526 159282 (view as bug list)
Depends on:
Blocks:
 
Reported: 2002-08-22 10:49 EDT by quartz quartz CLA
Modified: 2010-09-27 07:08 EDT (History)
33 users (show)

See Also:


Attachments
feature and preferences (51.67 KB, image/jpeg)
2003-06-21 15:42 EDT, Sebastian Davids CLA
no flags Details
feature and preferences for the JavaEditor (71.85 KB, image/jpeg)
2003-06-21 15:43 EDT, Sebastian Davids CLA
no flags Details
Addition to jface.text -- WhitespacePainter (5.66 KB, patch)
2003-06-21 16:08 EDT, Sebastian Davids CLA
no flags Details | Diff
Adjustments in ui.editors (8.42 KB, patch)
2003-06-21 16:10 EDT, Sebastian Davids CLA
no flags Details | Diff
Adjustments in ui.workbench.texteditor (4.85 KB, patch)
2003-06-21 16:10 EDT, Sebastian Davids CLA
no flags Details | Diff
Adjustments in jdt.ui (8.96 KB, patch)
2003-06-21 16:11 EDT, Sebastian Davids CLA
no flags Details | Diff
zoomed in example (2.63 KB, image/png)
2004-05-04 22:31 EDT, Vitaliy CLA
no flags Details
CDT implementation ported to Platform (20.04 KB, patch)
2006-11-14 06:24 EST, Anton Leherbauer CLA
no flags Details | Diff
Proposed implementation (33.72 KB, patch)
2006-11-20 10:48 EST, Anton Leherbauer CLA
no flags Details | Diff
Revised version of Anton's patch (37.90 KB, patch)
2006-11-21 13:10 EST, Dani Megert CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description quartz quartz CLA 2002-08-22 10:49:42 EDT
there should be a show invisible chars (like tabs and spaces). Allow to choose
the representing char, and its color.
Comment 1 Sebastian Davids CLA 2003-06-21 15:42:55 EDT
Created attachment 5265 [details]
feature and preferences

show whitespaces turned on and preferences page for the TextEditor
Comment 2 Sebastian Davids CLA 2003-06-21 15:43:28 EDT
Created attachment 5266 [details]
feature and preferences for the JavaEditor

show whitespaces turned on and preferences page for the JavaEditor
Comment 3 Sebastian Davids CLA 2003-06-21 16:08:47 EDT
Created attachment 5267 [details]
Addition to jface.text -- WhitespacePainter

New class.

MatchingCharacterPainter was a model for this class.
Comment 4 Sebastian Davids CLA 2003-06-21 16:10:33 EDT
Created attachment 5268 [details]
Adjustments in ui.editors
Comment 5 Sebastian Davids CLA 2003-06-21 16:10:56 EDT
Created attachment 5269 [details]
Adjustments in ui.workbench.texteditor
Comment 6 Sebastian Davids CLA 2003-06-21 16:11:19 EDT
Created attachment 5270 [details]
Adjustments in jdt.ui
Comment 7 Sebastian Davids CLA 2003-06-21 16:21:34 EDT
I decided against preferences for the different decorators for spaces, tabs, EOL.

I think it would clutter the UI.

It could be added by proving setter methods for fSpaceCharacter, fTabCharacter,
fEOLCharacter, fSystemEOLChar in WhitespacePainter and then creating all the
neccessary UI "plumbing" to hook them together with their preferences.

If desired by the UI team I will adjust my patch to include these.

@@@@

Issues:

The whitespace decorators will flicker when one enters text.

This is similar to the flicker of the various annotations, the squiggly
underlines for example.

I think it is a limitation of the current "decoration" support, but I might be
wrong ,)
Comment 8 quartz quartz CLA 2003-08-14 10:54:03 EDT
How is that feature going? Is there a target milestone?
Comment 9 Dani Megert CLA 2003-08-15 05:46:12 EDT
*** Bug 31900 has been marked as a duplicate of this bug. ***
Comment 10 Dani Megert CLA 2003-08-15 05:46:18 EDT
*** Bug 41247 has been marked as a duplicate of this bug. ***
Comment 11 Sebastian Davids CLA 2003-09-09 09:19:08 EDT
this could be related to bug 27642 -- the slow repaint could cause the flicker
in the fix I've supplied
Comment 12 Dani Megert CLA 2003-10-15 04:49:58 EDT
*** Bug 25428 has been marked as a duplicate of this bug. ***
Comment 13 Dani Megert CLA 2003-10-15 04:50:36 EDT
*** Bug 44708 has been marked as a duplicate of this bug. ***
Comment 14 Eric Estievenart CLA 2003-10-30 09:34:59 EST
Copying back the comments and ideas I put on #44708 (duplicated),
since it may interest you.
---------------
Hello there, it would be great if there was an editor option
which would enable the developpers to see the tabs and spaces
in the editors.

This is because sometimes both get mixed in a source, thus
annoying a lot of people, and sometimes leading in a
massive automatic reformat of a file and then huge merge conflicts.

If we had the possibility to see the spaces,
people would
- immediately see that they are doing wrong, and avoid submitting
- be able to perform localized cleanings instead of reformating a
  whole file.

It could be a very complex option set like :

Show tabs   []at beginning []in the middle []at end  of lines
Show spaces []at beginning []in the middle []at end  of lines
(Which would probably make the editor a bit less efficient)

Or a very simple set which I think would make 99.99% of people
happy, including me :
[] Make spaces visible
[] Make tabs visible
(and would not impact on the efficiency of the editor)

Cheers
--Steve
Comment 15 Dani Megert CLA 2004-03-19 04:07:52 EST
*** Bug 55105 has been marked as a duplicate of this bug. ***
Comment 16 Vitaliy CLA 2004-05-04 22:31:06 EDT
Created attachment 10276 [details]
zoomed in example

this is an zoomed in screenshow of how this feature is implemeted in
"Whitespace" plugin for jEdit.	Such feature would be very helpful for us
Python developers as well as many others.
Comment 17 Leopold Welsch CLA 2004-10-14 05:25:04 EDT
Will there be anything done for version 3.1? Currently I do not see it in there.

Is it possible to make as a general feature for the Editor, so non-JavaEditors
could also use it?
Comment 18 Dani Megert CLA 2004-10-14 05:50:02 EDT
>Is it possible to make as a general feature for the Editor, so non-JavaEditors
>could also use it?
That's why this feature is in Platform Text and not JDT Text ;-)
Comment 19 Stefan Rank CLA 2005-07-19 04:26:33 EDT
bump

(sorry, but

 - this seems to be more or less ready
 - no comments since october last year
 - it is a substantial feature for professional editing
 - i could not find a plugin that does it ;-)

cheers
Comment 20 Dani Megert CLA 2005-08-04 09:43:11 EDT
*** Bug 106038 has been marked as a duplicate of this bug. ***
Comment 21 Dani Megert CLA 2005-08-16 10:24:22 EDT
*** Bug 107076 has been marked as a duplicate of this bug. ***
Comment 22 Dani Megert CLA 2005-08-25 02:40:51 EDT
*** Bug 107936 has been marked as a duplicate of this bug. ***
Comment 23 Dani Megert CLA 2005-09-21 03:12:12 EDT
*** Bug 110091 has been marked as a duplicate of this bug. ***
Comment 24 Dani Megert CLA 2005-11-27 12:00:36 EST
*** Bug 118079 has been marked as a duplicate of this bug. ***
Comment 25 Philippe Ombredanne CLA 2006-01-31 12:56:23 EST
Status?
Is there a chance thatw e see this one in 3.2?
Thanks!
Comment 26 Dani Megert CLA 2006-02-01 03:08:26 EST
We will take a look during M6 (time permitting)
Comment 27 Dani Megert CLA 2006-02-01 03:08:51 EST
*** Bug 33829 has been marked as a duplicate of this bug. ***
Comment 28 Dani Megert CLA 2006-03-25 12:37:38 EST
I've looked at the patches and decided to move this to 3.3 with P2 priority:
- the WhitespacePainter only handles one kind of line delimiter but a document
  can have an arbitrary set of line delimiters
- new API is added through the patches - which is OK, but too late in the game.
  Instead of making this new functionality internal I prefer to put in a right
  solution for 3.3.
- the patches need to be adjusted to the new general text editor preferences and
  hence changes in JDT Text land won't be needed

David, could you adjust the patches for 3.2 and in addition, some tests would be great.
Comment 29 Dani Megert CLA 2006-06-08 05:04:41 EDT
David, can we expect and update for 3.3?
Comment 30 Sebastian Davids CLA 2006-06-08 09:06:46 EDT
Am I David? .)

I'll adjust them in a few days...
Comment 31 Dani Megert CLA 2006-06-08 09:20:49 EDT
yes - looks like my human bugzilla parsing needs work ;--)
Comment 32 Sebastian Davids CLA 2006-06-17 21:23:42 EDT
I've finished porting.

Haven't tested it on Windows yet, but on Mac OS X it's dog slow because of StyledText.getOffsetAtLocation.

Is there a better/faster way than:

IRegion region = sourceViewer.getVisibleRegion();

int offset = region.getOffset();
int len = region.getLength();

IDocument document = sourceViewer.getDocument();

for (int i = offset; i < len; i++) {
	char c = document.getChar(i);

	if (/* whitespace*/) {
		Point point = styledText.getLocationAtOffset(offset);
		gc.drawText(decoratorChar, point.x, point.y, true);
	}
}


Basically draw a decoratorChar at the same position (offset) as a whitespace char.

This performance problem is already mentioned in bug 27642.
Comment 33 Dani Megert CLA 2006-07-03 10:02:00 EDT
>This performance problem is already mentioned in bug 27642.
I verified the scenario in bug 27642 and it is no longer an issue in R3.2.

Do you need to do the looping and handle the drawing? Instead you could add your own custom annotations (that implement IAnnotationPresentation) and let the annotation painter framework handle the drawing.
Comment 34 Sebastian Davids CLA 2006-07-03 14:56:38 EDT
"annotation painter framework"

I'll look into that.
Comment 35 Kim Horne CLA 2006-10-02 11:43:27 EDT
*** Bug 159282 has been marked as a duplicate of this bug. ***
Comment 36 Andrey Loskutov CLA 2006-11-11 13:51:50 EST
Hi folks, I was not aware of this bug so that I've just implemented a simple but practical annotation-based solution which could show tabs and spaces (but not the line ends) for text editors (works with both Eclipse 3.1 and 3.2). 
This is now a part of AnyEdit tools plugin, see screenshots at:
http://andrei.gmxhome.de/anyedit/examples.html
and
http://andrei.gmxhome.de/anyedit/preferences.html

Of course having core platform text implementation would be great, so keep working on :o)

Regards, 
Andrei
Comment 37 Dani Megert CLA 2006-11-12 12:20:37 EST
Andrei, just wondering how well your code performs on large files?
Comment 38 Andrey Loskutov CLA 2006-11-12 12:36:38 EST
Daniel, how you define "large"? 
org.eclipse.ui.texteditor.AbstractTextEditor has ~5600 lines and it requires <1 sec in background job to annotate it on my Atlon64 X2 3800 MHz and 2 gig RAM. I think for average Java developer (as I'm) it is ok. In my code I do not maintain the current state of annotation model during source modification, it is enough to update it on save of dirty buffer. 
I could made some measurements if you like. 
Comment 39 Pavel Krupets CLA 2006-11-12 13:01:23 EST
Daniel, how you define "large"?  :)
What do you mean by < 1 sec. 0.99? :))
Comment 40 Andrey Loskutov CLA 2006-11-12 14:20:48 EST
@Daniel - how I can precisely measure *UI* thread (painting of annotations)? 

I've stopped time with org.eclipse.jdt.internal.compiler.parser.Parser from 3.2, it has 10023 lines and my code adds 35630 whitespace annotations to the model. Btw, I use Eclipse 3.2.1/WinXP/JDK 1.5.

The background job which parses code and adds annotations to the model was done in ~820 milliseconds (~44 new annotations per millisecond) (@Pavel - it is average time for 3 runs :).

I saw the *last* 5 lines annotated in UI after approx. additional 3 seconds. 
So the most time was spent by painting new annotations (1 vs 3 sec) in the UI thread (even if most of this annotations are invisible to user).

Do not forget, that in current code I annotate each single space inside of the line, not only leading or trailing whitespace. 

I've also tried with modified code which ignores single spaces, then with 9747 annotations time decreases to 200 ms/~1 sec for backgr/UI threads.

Using "squiggles" instead of "highlighting" for annotation seems to improve performance of UI thread. I have no numbers in this case because I do not know how to measure UI thread time.

BTW, *removing* annotation from model needs *much* longer time in UI thread as adding them ?!?
Hier is my code to throw my annotations away (extension is the IAnnotationModelExtension):

extension.removeAnnotationModel(ToggleWhitespace.class);
// to trigger repaint
extension.addAnnotationModel(ToggleWhitespace.class,  new AnnotationModel());
Comment 41 Anton Leherbauer CLA 2006-11-13 03:15:07 EST
FYI, the CDT 4.0 stream already has this feature. It is implemented as a lightweight painter and is usable on any Eclipse text editor.
Comment 42 Dani Megert CLA 2006-11-13 03:19:40 EST
>Daniel, how you define "large"?  :)
Parser and AbstractTextEditor are good examples for what I meant.

>@Daniel - how I can precisely measure *UI* thread (painting of annotations)? 
By using a profiler and looking at the UI thread.


Just to make sure I understood right: you only run the feature upon save? And during editing we see the old state, right?

>Using "squiggles" instead of "highlighting" for annotation seems to improve
>performance of UI thread.
That's pretty strange because highlighting should be done by the widget while squiggles are manually painted.

>// to trigger repaint
>extension.addAnnotationModel(ToggleWhitespace.class,  new AnnotationModel());
Better is to call yourModel.removeAllAnnotations() unless of course you want to have a new model installed anyway.
Comment 43 Dani Megert CLA 2006-11-13 03:29:48 EST
Anton,

is this done on typing or just saving? If so, how's performance doing? What out of  \t \n \r and SPACE does it support?
Comment 44 Leopold Welsch CLA 2006-11-13 03:35:41 EST
I looked on the examples and I find it as a good starting-point. 

Nevertheless showing white-spaces as coloured space is not the option why I am voting for this Entry. I would like more to have them visible similiar to Word/Write, e.g. Tab as some kind of Arrow, Space some kind of Dot, etc. Otherwise the UI gets too much colourful and disattracting.

@Anrdei: 
- Could this be done as well easily from your code?
- Is there any reason, why the CR/LF are not included?
Comment 45 Anton Leherbauer CLA 2006-11-13 04:27:07 EST
(In reply to comment #43)
> is this done on typing or just saving? If so, how's performance doing? 

It's done while typing. Performance is good, never heard a complaint (has been in our commercial product for several years now).

> What out of  \t \n \r and SPACE does it support?

all of them
Comment 46 Andrey Loskutov CLA 2006-11-13 16:13:44 EST
(in reply to comment 41)
If it is already a part of CDT 4, are there any plans to move it to the platform text, so that all users could finally have it, and not only C/C++ hackers :o) ?

(in reply to comment 42)
> during editing we see the old state
Yes, my use case was to "quickly show tabs vs spaces in editor", not to have permanent and consistent model during the document edit operations. Therefore the model synchronization happens on each save or on toggle. It was not intended to be a final soluton like CDT implementation, it is just something which I immediately could use and costs a week to implement :o)
Thanks for other suggestions, Daniel.

(in reply to comment 44)
- no, as it uses Annotations which are limited by "highlighting" and "squiggles". But squiggles are not far away from what you want :o)
- I didn't found a way to show Annotation on line end (*not* on the last char)
Comment 47 Dani Megert CLA 2006-11-14 02:35:34 EST
Anton would CDT be willing to contribute this to Platform Text and if so attach a patch here for review whether/how this would fit into the current Platform Text architecture and patterns?
Comment 48 Anton Leherbauer CLA 2006-11-14 06:24:02 EST
Created attachment 53802 [details]
CDT implementation ported to Platform

This patch adds a toolbar action "Show Invisible Characters" to the Editor Presentation action-set. (The icon is not included in the patch.)
It would be great to have this in the Platform instead of CDT.
Note, that there is a little hack, which should probably be fixed properly: To get the ITextViewer from an ITextEditor, I use getAdapter(ITextOperationTarget.class) and cast the result to ITextViewer. I'd suggest to make the AbstractTextEditor adaptable to ITextViewer directly, instead.
Another point is that the action does not support multi-part editors which contain a text editor (e.g. the PDE Manifest editor).
Comment 49 Andrey Loskutov CLA 2006-11-14 08:04:41 EST
(in reply to coment 48)
> Another point is that the action does not support multi-part editors 
> which contain a text editor (e.g. the PDE Manifest editor).

@Daniel: it is a bit offtopic, but why not allow multipage editors (MultiPageEditorPart class) to override getAdapter() and return getActiveEditor() in the case of IEditorPart.class argument. This would fix this nasty problem - currently it is (fast) inpossible to get a reference to ITextEditor if you have a MultiPageEditorPart active.
Comment 50 Andrey Loskutov CLA 2006-11-14 08:06:20 EST
Sorry, in previous comment one should read ITextEditor instead of IEditorPart
Comment 51 Dani Megert CLA 2006-11-14 08:13:13 EST
From looking at the code this should already work.
Comment 52 Dani Megert CLA 2006-11-16 05:23:53 EST
Looks nice and performance is good. I'd like to continue based on that patch.

There is a little problem when combined with folding: the folding indicator at the end of a line overlaps with the invisible character. I suggest to not paint the invisible char info on a folded line.

I'm pretty limited on resources and would be happy to get help on this. Could you submit a patch with the following changes/additions:
- solve the conflict with folding annotation
- convert the current global action to an TextEditorAction and add:
  - action id constant
  - command id (action definition) constant
  - help id constant
- mark the action in plugin.xml as retarget action
- register the action in the BasicTextEditorActionContributor (use similar
  pattern as used for JDT's TogglePresentationAction)
- add a preference to the 'Text Editors' preference page which would be in
  sync with the state of the toggle (needs a listener to update the action, again see TogglePresentationAction)
- add preference keyword(s) to plugin.xml and plugin.properties that allows to search for the new
  preference
- externalize all the strings
- confirm that the code which comes from CDT was originally developed by you
  or if not, give the right COO info

>Note, that there is a little hack, which should probably be fixed properly: To
>get the ITextViewer from an ITextEditor, I use...
For now simply check whether the editor is an AbstractTextEditor and if so call getSourceViewer(). We currently have several actions that aren't 100% compliant to ITextEditor.
Comment 53 Dani Megert CLA 2006-11-16 06:15:43 EST
I don't like the character used for LF. I suggest to use the more common 'u21B5' but this seems to confuse the painter and the invisible characters are painted a bit too high.
Comment 54 Dani Megert CLA 2006-11-16 09:20:28 EST
I used these, which use the corresponding Unicode character:

	private static final char MIDDLE_DOT= '\u00b7';
	private static final char TAB_SIGN= '\u21e5';
	private static final char CARRIAGE_RETURN_SIGN= '\u21b5';
	private static final char LINE_FEED_SIGN= '\u21b4';

We should also make sure that the Tab symbol is placed where the tab ends. Currently it is placed at the beginning.
Comment 55 Anton Leherbauer CLA 2006-11-16 10:37:54 EST
On which platform did you try these characters? On WinXP I can only see rectangles. I knew there was a reason I chose only Latin-1 characters ;-) 
To be honest, I chose the same characters that were (are) used in SNiFF+.

> We should also make sure that the Tab symbol is placed where the tab ends.
> Currently it is placed at the beginning.

I find it quite intuitive to have the tab displayed at the beginning. It could be hard to compute the correct location for the end (or center) position.

(In reply to comment 52)
> Could you submit a patch with the following changes/additions

I am also pretty busy, but I will try to make the changes asap.

> - confirm that the code which comes from CDT was originally developed by you
  or if not, give the right COO info

I hereby confirm that the code is originally developed by me.
Comment 56 Dani Megert CLA 2006-11-16 10:43:05 EST
I run the code on WinXP.
Comment 57 Dani Megert CLA 2006-11-17 04:30:17 EST
> I knew there was a reason I chose only Latin-1 characters ;-) 
My bad ;-)

We should try to find something inside Latin-1 that looks like a right arrow for the tab and a better one for the pilcrow which usually stands for paragraph or section i.e. it is not obvious whether this stands for \n or \r. I'm also not very fond of the LATIN_SMALL_LETTER_THORN character. Maybe we could use the logical not sign. Anyway - those are details we can settle while we go.
Comment 58 Sebastian Davids CLA 2006-11-17 06:11:17 EST
In my old version (comment 3) I used:

public class WhitespaceCharacters {

    public static void main(String[] args) {
        String spaceCharacter = String.valueOf((char) 183); // middot
        String tabCharacter = String.valueOf((char) 187); // >>
        String eolCharacter = String.valueOf((char) 182); // paragraph sign
        System.out.println(spaceCharacter);
        System.out.println(tabCharacter);
        System.out.println(eolCharacter);
    }
}
Comment 59 Sebastian Davids CLA 2006-11-17 06:30:37 EST
What about the european paragraph sign for LF?

http://www.fileformat.info/info/unicode/char/00a7/index.htm

        System.out.println('\u00A7');
        System.out.println(String.valueOf((char) 167));

Note:

Besides using this painter in a development environment where the distinction between /n and /r is important one might use it in a "business" environment where this is not important.

In other words: a "Word"-like App should not show two chars (one for \n and one for \r) on Windows but one (as an EOL sign).
Comment 60 Dani Megert CLA 2006-11-17 11:44:15 EST
This is a professional feature and hence should show all invisible whitespace chars. -1 for a "business" mode.
Comment 61 Leopold Welsch CLA 2006-11-20 06:16:47 EST
(In reply to comment #55)
> On which platform did you try these characters? On WinXP I can only see
> rectangles. I knew there was a reason I chose only Latin-1 characters ;-) 
> To be honest, I chose the same characters that were (are) used in SNiFF+.
> 
> > We should also make sure that the Tab symbol is placed where the tab ends.
> > Currently it is placed at the beginning.
> 
> I find it quite intuitive to have the tab displayed at the beginning. It could
> be hard to compute the correct location for the end (or center) position.

Completely agreed. Microsoft Office applications place the TAB-display at the beginning as well does it OpenOffice. So, why to invent new UI-designs which many users already used to? (Perhaps there might be a reason, nevertheless I would not vote to use new UI-styles which are not widely accepted.)

(In reply to comment #55)

I would treat them as well as somehow the standard-signs in Word and OOO- at least for space-Char and for the eol-char. The TAB-Sign is different, but here as well UltraEdit uses it.

So, why is there the need to reinvent the UI-design?
Comment 62 Anton Leherbauer CLA 2006-11-20 09:57:13 EST
I have an implementation question: I tried to mimic the TogglePresentationAction, but I don't know where to get the preference store from. EditorsUI.getPreferenceStore() is not accessible from the org.eclipse.ui.workbench.texteditor plugin where BasicTextEditorActionContributor is defined. Ie. I would need to move the action to org.eclipse.ui.editors or there is another way of accessing the preference store.

Another question is whether the painter should be active on the current active editor only, or on all editors. The original implementation sets the painter to the current active editor only, ie. when editors are open side-by-side, only one of them (the active one) shows the invisible characters.
Associating the action to an IEditorActionBarContributor would change this to "the last active one of the same kind" which is possibly confusing.
Comment 63 Dani Megert CLA 2006-11-20 10:22:44 EST
The action would be created in AbstractTextEditor.createActions() which has access to the editor's preference store.

>Associating the action to an IEditorActionBarContributor would change this to
>"the last active one of the same kind" which is possibly confusing.
No it would not. The editors would listen to preference changes and hence also update when the preference changes. They should all be in sync.

I can live with the Tab sign '>>' at the beginning. UltraEdit indeed does it like that.

>So, why is there the need to reinvent the UI-design?
Because Word does not show CR and LF but has the pilcrow sign for paragraphs and the down-left arrow for simple manual line breaks. When using the Text or Java editor there are is no such concepts as paragraphs. Only showing the pilcrow sign as UltraEdit does is pretty useless because currently the Text and Java editor do not support auto-wrapping and hence all visible lines end with some sort of line delimiters and would get the pilcrow. I want to see CR and/or LF.
Comment 64 Anton Leherbauer CLA 2006-11-20 10:39:04 EST
(In reply to comment #63)
> The action would be created in AbstractTextEditor.createActions() which has
> access to the editor's preference store.
Yes, but only to the preference store that the editor receives from the concrete subclass, which may also be a read-only ChainedPreferenceStore (e.g. JDT and CDT). When the action should be able to change the preference, then it must have access to the writable preference store.

> 
> >Associating the action to an IEditorActionBarContributor would change this to
> >"the last active one of the same kind" which is possibly confusing.
> No it would not. The editors would listen to preference changes and hence also
> update when the preference changes. They should all be in sync.

Ok, then this is actually not the pattern of TogglePresentationAction.

Before we get into too much theoretical discussion, I'll post a patch shortly to give it a bit more substance.
Comment 65 Anton Leherbauer CLA 2006-11-20 10:48:09 EST
Created attachment 54175 [details]
Proposed implementation

This implementation creates the action in the AbstractDecoratedTextEditor (similar to the BooleanPreferenceToggleAction), but as the action attaches a preference store listener which would never be removed, this is handled by calling setEditor(null) in the editor's dispose method.
Comment 66 Dani Megert CLA 2006-11-20 10:49:03 EST
You're right the store is small problem. I suggest to write an action that takes the store. in AbstractTextEditor.createActions() you use the given store and in AbstractDecoratedTextEditor.createActions() we create the action with EditorsUI store (overwriting the one from AbstractTextEditor).

>Ok, then this is actually not the pattern of TogglePresentationAction.
The action toggles the state for all Java editors.
Comment 67 Dani Megert CLA 2006-11-21 13:10:34 EST
Created attachment 54273 [details]
Revised version of Anton's patch

Anton, I like the patch and it is almost ready to be committed. Instead of listing all my comments I crafted an updated version of your patch which for example fixes the not working preference. I also replaced 'invisible characters' wording with 'whitespace characters'. Let me know whether it is OK for you to commit that.
Comment 68 Anton Leherbauer CLA 2006-11-21 15:30:08 EST
I am perfectly OK with this revised patch. Thanks!
I would only suggest to use a larger icon, it is a bit small.
I tried attaching the icon from CDT, but bugzilla won't let me do it. Will try again later.
Comment 69 Dani Megert CLA 2006-11-22 05:13:23 EST
This has been released to HEAD.
Available in builds > N20061122-0800.

We might still fine tune the used character for CR. Other candidates might be:
  http://www.fileformat.info/info/unicode/char/00a7/index.htm
  http://www.fileformat.info/info/unicode/char/00ac/index.htm
and other suggestions are also welcome.

I filed a new bug 165429 to get and install the final tool bar icon.
Comment 70 Anton Leherbauer CLA 2006-11-22 06:51:23 EST
> This has been released to HEAD.
Great! Thank you!

I like the currency symbol ('¤') for CR more than the logical not ('¬') or the section sign ('§'), but I don't have a very strong opinion on that.
Comment 71 Leopold Welsch CLA 2006-11-22 10:55:45 EST
Only one question to Daniel:

Which well-known programms do use 'a7' and 'ac'? I know only the Office-Programms from Microsoft and OOo and they use the Pilcrow. So, I would vote for a widely used sign which is used as well by many other programs.
Comment 72 Dani Megert CLA 2006-11-22 11:07:06 EST
We DO use the pilcrow sign for LF but there's also CR under Windows.
Comment 73 Mark A. Ziesemer CLA 2006-11-27 14:45:26 EST
First, THANK YOU for resolving this bug - it was definitely one of my most-missed features in Eclipse!  I incorporated the last patch into my 3.3M3 install without any issues, other than the GIF icon - gets a little corrupted in a text-based patch.  :-)  (Which is easily fixed by downloading the icon from bug 165429.)

== Alpha (color) selection? ==
What would everyone think about allowing the user to select the alpha of the whitespace characters?  I see that alpha is being used rather than color, which I completely agree with, as whitespace within different colors of text remains the given color, just "lighter".

Since we can already customize all the other colors used, it might be beneficial to customize this alpha, too.  For example, while I'm sure I will generally leave the whitespace characters shown, I may want to make them a little less noticeable without turning them off.  (Though I do like 100 for the default!)

It looks like this is currently specified by "gc.setAlpha(100);" at line 138 in my patched InvisibleCharacterPainter.java (against 3.3M3).  I could see this fitting quite well on the preference page, right next to the "Show whitespace characters" checkbox, just like the "print margin column" and "hyperlink style navigation key modifier" textboxes.
Comment 74 Mark A. Ziesemer CLA 2006-11-27 15:13:21 EST
Another relatively minor issue to be aware of: The scrollbar doesn't respect the line-ending whitespace characters when shown.  For example, if I end a line with 10 spaces, followed by a return (CR+LF), I can only scroll over enough to see the 10 spaces - I can't be sure whether the line ends with a CR+LF or otherwise.
Comment 75 Dani Megert CLA 2006-11-28 02:36:36 EST
>I can't be sure whether the line ends with a CR+LF or otherwise.
Correct since the symbols are painted and not part of the text.

NOTE: This bug is marked fixed. Please open separate bugs/feature request for each item.
Comment 76 Amy Wu CLA 2007-09-13 13:21:24 EDT
*** Bug 152526 has been marked as a duplicate of this bug. ***