Add option to include C0 and C1 characters to menu item Show NPC

[ozone10]

separate unicode EOL from main NPC list
add additional characters to main NPC list

fix #8111

[ozone10]

@alankilborn @Gitoffthelawn
could you test?

[alankilborn]

Not quite sure what I did to achieve the state shown below, but before I got to it, I was messing around choosing different encodings:

[ozone10]

@alankilborn
could you check if it's because of changing between ANSI and other encoding?
Might also reset current NPC appearance in master.

I am currently away so I won't be able to fix it soon.

[alankilborn]

@ozone10

It could be related to changing encodings but at the time of the screenshot above, encoding was UTF-8 (sorry, did not have status bar in the screenshot).

Also, would it be possible to change the text from Include "Cc" to Include C0/C1? To me, before I saw the popup hint text, Cc was a mystery. Popup hint text is fine, but if the control's text itself can stand alone, I think that is better.

[ozone10]

@alankilborn
it should not matter which encoding you currently have, only if in past you changed it to ANSI, which might reset npc representations and thus unsync settings.

In unicode they are called cc, but I can change it to Include C0, C1 ..., but later I am currently busy.

[donho]

@ozone10
Thank you.
It's no doubt a good feature. However, the scope of this PR and the used terms (regarding the scope) are not clear to me.
Also, I'm wondering the new GUI (with added checkbox) could bring some confusion.

But let's talk about the scope and terms firstly:
If this PR fix #8111, the scope of the PR should be only C0 codes (the ASCII range 0x00–0x1F, which are hidden well by the PR).
In the zone marked as C1 codes (in the PR) is not ASCII, but they are Unicode - according Wikipedia , they are codes ASCII range 0x80–0x9F. Or am I missing somethings?

[ozone10]

@donho
C1 can be used in other encodings not only in unicode
I mainly used https://www.scintilla.org/ScintillaDoc.html#CharacterRepresentations as reference

About checkbox, personally I prefer that C0/C1 codes to be shown regardless of state of "Show NPC" menu item, but some users might want to have them hidden, they want some kind of "reader mode".

[donho]

@ozone10

I mainly used https://www.scintilla.org/ScintillaDoc.html#CharacterRepresentations as reference

OK, in this case, is it better to use the term more generic "Control characters" instead of "C0 & C1" which is obscure for the most of users (it's obscure even to me)?

About checkbox, personally I prefer that C0/C1 codes to be shown regardless of state of "Show NPC" menu item, but some users might want to have them hidden, they want some kind of "reader mode".

Agree. I think it's good to provide an option to hide those characters and it's better to show them by default - that's the point of eventual confusion brought by new GUI I mentioned above:
Control characters set is different category from NPC's one. So instead of making them to join into NPC group by using a GUI option, I would add a new command Show Control Characters under the command Show Non-Printing Characters - it's more straight forward for users.
What do you think?

[ozone10]

@donho
C0 and C1 are subset of control characters.
Users should know what these characters are, if they want them hidden.
Because of limited space I also provide hint via tooltip.

To reuse same settings as current NPC (appearence), I didn't create separate command.
I also think that users will just set it once and forget.

Currently I have only fixed issue with npc state unsync when changing encodings.

If you still want to separate them to two commands, then maybe next week I will look into it.
For now I am unfortunately to busy with IRL.

Or if you want to implement it self, feel free, it should just require to duplicate current npc implementation with little tweaks.

[Yaron10]

@ozone10,

Thank you for your work. 👍

Could you please list the exact STR 52366d0 should fix?

---

Copy & paste the following text in a UTF-8 file.

Test​
Test​

Convert to ANSI.
Open a new UTF-8 file.
Paste.

Result:

---

Copy & paste the following text in a UTF-8 file.

Test​

Switch to another tab.
Switch back to the previous tab.

Result:

[Yaron10]

Convert to ANSI.
Open a new UTF-8 file.
Paste.

#11430.

[ozone10]

@Yaron10

Copy & paste the following text in a UTF-8 file.
Switch to another tab.
Switch back to the previous tab.

It should fix this.

for #11430 no.

Commit 52366d0 fix only npc representation unsync with menu item "Show Non-printing Characters" when changing encoding, as changing encoding could reset representation resulting in #13324 (comment)

PR does not touch data, only their appearance.

[Yaron10]

@ozone10,

Copy & paste the following text in a UTF-8 file.
Switch to another tab.
Switch back to the previous tab.

It should fix this.

This is still reproducible with the latest commit.
(NOTE: have only one line in the document).

for 11430 no.

After realizing it was caused by 11430, I understood it had nothing to do NPC.

Commit 52366d0 fix only npc representation unsync with menu item "Show Non-printing Characters" when changing encoding, as changing encoding could reset representation resulting in #13324 (comment)

Should I be able to reproduce the "npc representation unsync with menu" case with the current master?
Or was it caused by the first commit here?

Thank you.

[ozone10]

@Yaron10

This is still reproducible with the latest commit. (NOTE: have only one line in the document).

This is interesting representation are still active but they use original character width.

If you do some edits in text or some change which trigger redraw it might fix it and use correct width.

btw "SOH" might appear not affected, but if you use codepoint representation, it will be also clipped.

Should I be able to reproduce the "npc representation unsync with menu" case with the current master? Or was it caused by the first commit here?

I have not tested, but this issue should also affect current master.

[Yaron10]

@ozone10,

You've deleted some lines from your original reply. :)
Should I assume you have some idea how to fix it?

Thank you.

[Yaron10]

@ozone10,

👍
Thank you for the quick and interesting fix.

[Yaron10]

@ozone10,

There's still a problem with one-line.

Uncheck Display NPC.
Paste the one-line.
Check Display NPC.

Result:
NPC are invisible.

[ozone10]

@Yaron10
I may be wrong but this seems like scintilla issue

width is not updated for one line documents when using https://www.scintilla.org/ScintillaDoc.html#SCI_SETREPRESENTATION

commit 4976979 takes advantage of function (which is applied after "show npc" function) which has redraw property, so it fixed issue when changing tabs.

Uncheck Display NPC.
Paste the one-line.
Check Display NPC.

But here there is no such function to use.
I am not sure how to fix it without use of some function with redraw with no effects.

[Yaron10]

@ozone10,

Thank you for the explanation. 👍

The best applications have some "known-issues".
Should I open an issue for that minor problem?

[Yaron10]

@ozone10,

I'm not sure it's worth it, but wrapping/unwrapping solves the issue.
Something similar to:

notepad-plus-plus/PowerEditor/src/NppCommands.cpp

Line 3666 in 816fa3e

bool isWraped = _pEditView->isWrap();

[Yaron10]

@ozone10,

Thank you for the nice fix. 👍

Unfortunately, I've just noticed that the problem might occur even if the file contains more than one line.
Please download these files and open them in NPP.
Uncheck Show NPC.
Move T2 to the other view.
Check Show NPC.

Result (reproducible 9 out of 10 times):

How about using showEOL(isShownEol()); instead of redraw(); if willBeShowed == true in showNpc()?

[Yaron10]

@ozone10,

Great. 👍
Thanks again for your work.

[donho]

@ozone10
The command "Include C0, C1 and Unicode EOL in Non-Printing Characters" among "Show XXXX" commands is confusing to users IMO.

OK, I will do my version by inspiring your PR. Thank you for your coding effort.

[ozone10]

@donho
maybe adding separator and rewording it to "Use Non-Printing Characters Settings for C0, C1 and Unicode EOL" could improve it"

But as I said I am currently busy, so feel free to do necessary modifications.

[Explorer09]

Excuse me, but I have questions about the feature:

1. While there are reasons for hiding characters like ZWSP, what's the motive for hiding the C0 and C1 control characters at all? In my opinion, C0 and C1 control characters except whitespace characters should never be hidden, because in the security perspective hiding characters is generally bad as it open the potential of spoofing (Unicode has an technical report talking about security). While a sequence of SOH characters ("SOH SOH SOH SOH"), for example, may distract you visually a lot, they are visible for a reason. The alternative I can think of is to use U+2400 - U+241F character for displaying C0 control code or use U+FFFD (�), but they should never be hidden.
2. Does Notepad++ present a visual warning like this if there are invisible characters hidden in text documents and users chooses not to show them?
   
   (screenshot from this web page)
   I am referring to the nicknamed "Trojan Source" attack that IDEs such as Visual Studio Code mitigated with the visual warning above (note the exclamation signs ⚠ on the left of the line numbers). It's relevant to this Notepad++, you know.

[donho]

@ozone10
I found the GUI is more intuitive than the 1st version.
Once v8.5.2 RC is out, I will be back to this PR.
Thank you for the modification.

[ozone10]

@donho
Ok.
PR still needs some feedback:

• interaction with "Show all" menu/toolbar button
  currently it will show all characters when switching on, when switching off it doesn't hide C0, C1 and Unicode EOL
• persistent setting
  currently at Notepad++ start C0, C1 and Unicode EOL will be always shown

[donho]

@ozone10

3 quick questions/remarks:

BEFORE RENDERING

AFTER RENDERING

Is it intentional? If yes, why?

interaction with "Show all" menu/toolbar button
currently it will show all characters when switching on, when switching off it doesn't hide C0, C1 and Unicode EOL

For me, it's even more confusing. The "non rendered" looks like I or l.

2. I don't think "Render" is right term, instead of "Show".

3. The option of Control Characters in Preferences is not intuitive.

[donho]

@Explorer09

I am referring to the nicknamed "Trojan Source" attack that IDEs such as Visual Studio Code mitigated with the visual warning above (note the exclamation signs ⚠ on the left of the line numbers). It's relevant to this Notepad++, you know.

@ozone10
OTOH, instead of using confusing characters for representing hidden Control Characters, I think it's better to display a similar warning to users. It could be a thread to scan the whole document and give the warning - but it will be another feature.

[ozone10]

@donho
I have partially reverted previous commit.

1. ...

Currently "Show C0, C1 ..." is fully separated from "Show All" command

2. I don't think "Render" is right term, instead of "Show".

I used term "render" to distinguish it more from other "show" command, IIRC it is used in some other text editors.

3. The option of Control Characters in Preferences is not intuitive.

There is also tooltip for it, but do you have better wording or idea for it

OTOH, instead of using confusing characters for representing hidden Control Characters, I think it's better to display a similar warning to users. It could be a thread to scan the whole document and give the warning - but it will be another feature.

Using warning should be better👍.

[donho]

Please consider the GUI suggestions and modify concerning data

[donho]

Please do the following instead:

        BEGIN
            MENUITEM "Show Space and Tab",                     IDM_VIEW_TAB_SPACE
            MENUITEM "Show End of Line",                       IDM_VIEW_EOL
            MENUITEM "Show Non-Printing Characters",           IDM_VIEW_NPC
            MENUITEM "Show Control Characters && Unicode EOL", IDM_VIEW_NPC_CCUNIEOL
            MENUITEM "Show All Characters",                    IDM_VIEW_ALL_CHARACTERS
            MENUITEM SEPARATOR
            MENUITEM "Show Indent Guide",                      IDM_VIEW_INDENT_GUIDE
            MENUITEM "Show Wrap Symbol",                       IDM_VIEW_WRAP_SYMBOL
        END

[ozone10]

@donho
currently it is not part of "Show All Characters" won't it causes confusion if it is next to "Show All Characters" and not separated. Or should I include it in "Show All Characters".

[donho]

@ozone10
If "Show All Characters" shows NPC, I don't see why it doesn't show Control Characters & Unicode EOL.
So please include c0, c1 & Unicode EOL in "Show All Characters".

[donho]

Please do the following instead:

    CONTROL         "Apply to C0, C1 && Unicode EOL",IDC_CHECK_NPC_INCLUDECCUNIEOL,"Button",BS_AUTOCHECKBOX | WS_TABSTOP,17,62,140,10