#1129 Allow setting of color for control characters (STYLE_CONTROLCHAR)

[0xdeadbeef]

Apparently it's not possible to set the color for control characters (e.g. CR/LF) because the color attribute is not used for STYLE_CONTROLCHAR. However for a less distracting CR/LF display, it would be helpful to be able to set the color to one that is different from the normal text.

However, I'd also suggest to rework the CR/LF characters to a "nicer" look and/or add an option to just display a "paragraph" symbol as this is done in the Eclipse editor (or UltraEdit) instead of separate CR/LF symbols. While it can make sense to display CR/LF explicitly, most of the times only the general paragraph information is needed.

[Neil Hodgson]

I thought that a combined CRLF symbol may be an OK option but the mailing list opinion on this was negative.

I won't be working on either of these.

[0xdeadbeef]

Well, a combined "parapgraph" symbol works perfectly well in the Eclipse editor (and others like UEdit) and honestly the curent display of CR/LF symbols with filled black background looks absolutely horrible and unprofessional. I can understand that (at least as long as the hex editor plugin is only so-so) some people want an explicit CR/LF display (and the current symbols might serve as a stopgap), but combining them into one paragraph symbol at least as an option seems like a natural choice.
It's a bit beyond me why an otherweise brilliant editing component would deliberately choose not to implement it. Given that there are no important architectural reasons that would forbid this of course.

[Neil Hodgson]

Implementation requires time and effort which has to be performed by someone. Possibly others will be interested in working on this. To me, there are plenty of issues that are more interesting or beneficial than this one.

[WarrantyVoid]

Not being able to draw CR/LF symbols in a color which is less prominent than actual text is quite irritating for users and has been annoying me for ages. I cannot comprehend why this is not considered beneficial enough to fix. I do prefer the CR<->LF distinction over the Eclipse approach though.

[Werner Wernerschka]

I support this feature request very much. I'm using notepad++ which relies on scintilla and often use the option to display End of Line characters. But right now the style of the CR LF characters and also other non printable characters is much to intensive. The "white letter within black background"-style is quite irritating.

People using Notepad++ (and other scintilla depended Editors) would be glad if the style of those non printable characters could be individually changed. So we would ver much apppreciate if you could think about improving scintilla with this (not too difficult to implement?) feature request. Thank you very much!

[chavocarlos]

Hope that this feature could be done, the actual colors of the EOL CR LF character is too distracting, maybe just use the same color but using some opacity.

[Doğan Çelik]

I use Notepad++ and I support this feature. I commented about it here

[Igor]

Currently, this is really the most disturbing issue in Notepad++, IMHO.

[Neil Hodgson]

SCI_SETREPRESENTATIONAPPEARANCE and SCI_SETREPRESENTATIONCOLOUR added in [5ac616].

[Zufu Liu]

Optimized std::map updating for SpecialRepresentations and PropSetSimple.

PropSetSimple::GetInt() and Accessor::GetPropertyInt() can change key to std::string_view as all lexer properties are string literal.

[Neil Hodgson]

Doesn't the changed PropSetSimple::Set return true even when the value is the same?

[Zufu Liu]

!updated && it->second != val

[Neil Hodgson]

If its worth the extra complexity of emplace then its worth looking at the actual hot path which will be for an existing key (there's a limited set of properties) and for the same value as presently set. According to https://en.cppreference.com/w/cpp/container/map/emplace,

The element may be constructed even if there already is an element with the key in the container, in which case the newly constructed element will be destroyed immediately.

Performing a find first then checking that for an early return would avoid some construction. Perhaps realize the key string_view into a string so it can be reused if inserting is required.

[Neil Hodgson]

Changed PropSetSimple so all key arguments are string_view with https://github.com/ScintillaOrg/lexilla/commit/f9c6debea7296f598c9b7b324fad426afb2fa54c

[Zufu Liu]

Changed to https://en.cppreference.com/w/cpp/container/map/try_emplace and https://en.cppreference.com/w/cpp/container/map/insert_or_assign

[Neil Hodgson]

Ran benchmarks on PropSetSimple::Set for

1) the original code (which doesn't report update so not equivalent),
2) PropSetSimple.diff with &&
3) PropSetSimple-2.diff
4) Using find
5) Redeclaring mapss with a compare argument so it can be called with std::string_view then calling it with string_view.

The results are (shaving off last 3 digits and superfluous text):
64-bit Release Visual C++ 16.10.2

Code	Time
1	26539
2	42031
3	26254
4	23352
5	11115

32-bit Release Visual C++ 16.10.2

Code	Time
1	26194
2	41928
3	25799
4	24793
5	9905

The code for PropSetSimple::Set:

#define IMPL 1

#if IMPL == 5
typedef std::map<std::string, std::string, std::less<>> mapss;
#else
typedef std::map<std::string, std::string> mapss;
#endif
// ...
bool PropSetSimple::Set(std::string_view key, std::string_view val) {
    mapss *props = PropsFromPointer(impl);
    if (!props)
        return false;
#if IMPL == 1
    (*props)[std::string(key)] = std::string(val);
    // Original code, doesn't return status
    return false;
#elif IMPL == 2
    auto [it, updated] = props->emplace(key, val);
    if (!updated && it->second != val) {
        it->second = val;
        updated = true;
    }
    return updated;
#elif IMPL == 3
    auto [it, inserted] = props->try_emplace(std::string(key), val);
    if (!inserted && it->second != val) {
        it->second = val;
        inserted = true;
    }
    return inserted;
#elif IMPL == 4
    mapss::iterator it = props->find(std::string(key));
    if (it != props->end()) {
        if (val == it->second)
            return false;
        it->second = val;
    } else {
        props->emplace(key, val);
    }
    return true;
#elif IMPL == 5
    mapss::iterator it = props->find(key);
    if (it != props->end()) {
        if (val == it->second)
            return false;
        it->second = val;
    } else {
        props->emplace(key, val);
    }
    return true;
#endif
}

Benchmark (using catch.hpp for timing):

constexpr int repetitions = 10000000;

constexpr const char *propertyNames[] = {
    "styling.within.preprocessor",
    "lexer.cpp.allow.dollars",
    "lexer.cpp.track.preprocessor",
    "lexer.cpp.update.preprocessor",
    "lexer.cpp.verbatim.strings.allow.escapes",
    "lexer.cpp.triplequoted.strings",
    "lexer.cpp.hashquoted.strings",
    "lexer.cpp.backquoted.strings",
    "lexer.cpp.escape.sequence",
    "fold",
    "fold.cpp.syntax.based", 
    "fold.comment",
    "fold.cpp.comment.multiline",
    "fold.cpp.comment.explicit",
    "fold.cpp.explicit.start", 
    "fold.cpp.explicit.end",
    "fold.cpp.explicit.anywhere",
    "fold.cpp.preprocessor.at.else",
    "fold.preprocessor",
    "fold.compact",
    "fold.at.else",
};

void Setter() {
    PropSetSimple pss;
    Catch::Timer tikka;
    tikka.start();
    for (int rep = 0; rep < repetitions; rep++) {
        for (size_t n = 0; n < std::size(propertyNames); n++) {
            const char *name = propertyNames[n];
            if (n % 10 == 0) {
                // 10% choose a different value
                pss.Set(name, "0");
            } else {
                pss.Set(name, "1");
            }
        }
    }
    std::cout << "Speed of set " << tikka.getElapsedMicroseconds() << " microseconds" << '\n';
}

[Neil Hodgson]

Some changes with https://github.com/ScintillaOrg/lexilla/commit/aaefcf15828605e219bb411e8dc8a78cfa65e35b

[Neil Hodgson]

Changed SetRepresentationAppearance and SetRepresentationColour but not SetRepresentation with [757e44] .

[Zufu Liu]

diff -r b152bb68c584 src/PositionCache.cxx
--- a/src/PositionCache.cxx Mon Jun 21 13:35:09 2021 +1000

SpecialRepresentations::SetRepresentation() can be changed to try_emplace():
+++ b/src/PositionCache.cxx Tue Jun 22 23:01:48 2021 +0800
@@ -548,13 +548,14 @@
 void SpecialRepresentations::SetRepresentation(std::string_view charBytes, std::string_view value) {
    if ((charBytes.length() <= 4) && (value.length() <= Representation::maxLength)) {
        const unsigned int key = KeyFromString(charBytes);
-       MapRepresentation::iterator it = mapReprs.find(key);
-       if (it == mapReprs.end()) {
+       const auto [it, inserted] = mapReprs.try_emplace(key, value);
+       if (inserted) {
            // New entry so increment for first byte
            const unsigned char ucStart = charBytes.empty() ? 0 : charBytes[0];
            startByteHasReprs[ucStart]++;
+       } else {
+           it->second = Representation(value);
        }
-       mapReprs[key] = Representation(value);
    }
 }