D17241: WIP:Disable highlighting after 512 characters on a line.

sars added a comment. I think a partially highlighted line is better than a totally non-highlighted one. And I think that the user is more likely to instinctively guess correctly why the end of the line is not highlighted than if the line is not highlighted at all. REPOSITORY R39

D17241: WIP:Disable highlighting after 512 characters on a line.

cullmann added a comment. Hmm, would a partially highlighted line really preferable? REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D17241 To: cullmann, vkrause, dhaumann, mwolff, sars Cc: zetazeta, mwolff, brauch, kwrite-devel, kde-frameworks-devel, LeGast00n,

D17241: WIP:Disable highlighting after 512 characters on a line.

mwolff added a comment. I personally dislike this too. INLINE COMMENTS > katerenderer.cpp:408 > +auto = renderRanges.pushNewRange(); > for (int i = 0; i < al.count(); ++i) > if (al[i].length > 0 && al[i].attributeValue > 0) { couldn't the al.count be limited

D17241: WIP:Disable highlighting after 512 characters on a line.

cullmann added a comment. 4daee27406cad0001f30bca4bf551495e0624d23 > = - addConfigEntry(ConfigEntry(LineLengthLimit, "Line Length Limit", QString(), 4096)); +

D17241: WIP:Disable highlighting after 512 characters on a line.

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit R39:1b7a2c69e492: WIP:Disable highlighting after 512 characters on a line. (authored by cullmann). REPOSITORY R39

D17241: WIP:Disable highlighting after 512 characters on a line.

cullmann added a comment. Then let's try that. Even for all cases without hitting the limit the new code is faster. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D17241 To: cullmann, vkrause, dhaumann, mwolff, sars Cc: zetazeta, mwolff, brauch, kwrite-devel,

D17241: WIP:Disable highlighting after 512 characters on a line.

dhaumann accepted this revision. dhaumann added a comment. +1 REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D17241 To: cullmann, vkrause, dhaumann, mwolff, sars Cc: zetazeta, mwolff, brauch, kwrite-devel, kde-frameworks-devel, LeGast00n, domson, michaelh,

D17241: WIP:Disable highlighting after 512 characters on a line.

cullmann added a comment. I am not sure about the warning. The line length limit default should be increased it think, we could just try something like 1 for a start. I would propose we try this without a warning and see if we get negative feedback. Normally this should only kick

D17241: WIP:Disable highlighting after 512 characters on a line.

sars added a comment. I tried the patch and it improved the performance really much! :) I was able to edit a line that contained over a million characters! Unfortunately I can't comment on the code too much, as I don't know the code enough. Some general stuff: 1. Should there

D17241: WIP:Disable highlighting after 512 characters on a line.

cullmann added a comment. For the outer vector, yes. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D17241 To: cullmann, vkrause, dhaumann, mwolff, sars Cc: zetazeta, mwolff, brauch, kwrite-devel, kde-frameworks-devel, LeGast00n, domson, michaelh, ngraham,

D17241: WIP:Disable highlighting after 512 characters on a line.

dhaumann added a comment. Could this be pushed even further by using QVarLengthArray? REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D17241 To: cullmann, vkrause, dhaumann, mwolff, sars Cc: zetazeta, mwolff, brauch, kwrite-devel, kde-frameworks-devel, LeGast00n,

D17241: WIP:Disable highlighting after 512 characters on a line.

cullmann updated this revision to Diff 59550. cullmann added a comment. Remove a MASS of heap allocations. We allocated the range on the heap and then inside we really even allocated all 16 byte KTextEditor::Range objects again on the heap. REPOSITORY R39 KTextEditor CHANGES SINCE LAST

D17241: WIP:Disable highlighting after 512 characters on a line.

cullmann edited the summary of this revision. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D17241 To: cullmann, vkrause, dhaumann, mwolff, sars Cc: zetazeta, mwolff, brauch, kwrite-devel, kde-frameworks-devel, LeGast00n, domson, michaelh, ngraham, bruns, demsking,

D17241: WIP:Disable highlighting after 512 characters on a line.

cullmann marked an inline comment as done. cullmann added a comment. Here is my try on this. Instead of limiting the line length, we just limit the number of attributes we are allowed to display. That only hits stuff != selection. e.g. for the example out of

D17241: WIP:Disable highlighting after 512 characters on a line.

cullmann updated this revision to Diff 59548. REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17241?vs=48461=59548 REVISION DETAIL https://phabricator.kde.org/D17241 AFFECTED FILES src/render/katerenderer.cpp To: cullmann, vkrause, dhaumann, mwolff,

D17241: WIP:Disable highlighting after 512 characters on a line.

cullmann commandeered this revision. cullmann edited reviewers, added: sars; removed: cullmann. cullmann added a comment. I have drafted a small limit to attributes like I proposed. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D17241 To: cullmann, vkrause,

D17241: WIP:Disable highlighting after 512 characters on a line.

cullmann requested changes to this revision. cullmann added a comment. This revision now requires changes to proceed. I think we shall count the number of ranges on the line to judge the limit, otherwise this is a sound idea. REPOSITORY R39 KTextEditor REVISION DETAIL

D17241: WIP:Disable highlighting after 512 characters on a line.

cullmann added a comment. Sorry to chime in that late ;=) Wouldn't it make more sense, to limit the number of ranges per line? I think the real issue is that we can't handle highlightings per line, not that the raw number of letters is that much an issue. This would relax the

D17241: WIP:Disable highlighting after 512 characters on a line.

sars added a comment. @dhaumann OK the limit is too low for Kile that is clear. Visual Studio Code is limiting the highlighting on a line to 1 characters. I tried to set the limit to 1, but that was very noticeably slow. Selecting a whole line took multiple seconds, which is

D17241: WIP:Disable highlighting after 512 characters on a line.

dhaumann added a comment. I'm not convinced this is a good idea: Kile users writing LaTeX tend to write one paragraph into a single line. They easily hit the 512 character limit. With this patch, we'd break syntax highlighting (completely?) for Kile. Stupidly asking: Can't this be

D17241: WIP:Disable highlighting after 512 characters on a line.

sars marked an inline comment as done and an inline comment as not done. sars added a comment. The new limit is now limiting the number of characters highlighted on a line. It is only the rest of the line that is not highlighted. This is IMO nicer as you don't see the non-highlighted part at

D17241: WIP:Disable highlighting after 512 characters on a line.

sars retitled this revision from "WIP:Disable highlighting for lines longer than 1024 characters." to "WIP:Disable highlighting after 512 characters on a line.". sars edited the summary of this revision. sars edited the test plan for this revision. REPOSITORY R39 KTextEditor REVISION DETAIL