D17241: WIP:Disable highlighting for lines longer than 1024 characters.
sars updated this revision to Diff 48461. sars added a comment. Disable highlighting after [limit] characters on a line in stead of the whole line. REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17241?vs=47146=48461 BRANCH disable_hl2 REVISION DETAIL https://phabricator.kde.org/D17241 AFFECTED FILES src/buffer/katetextbuffer.cpp src/document/katedocument.cpp src/render/katerenderer.cpp src/render/katerenderer.h src/utils/kateconfig.cpp src/view/kateviewhelpers.cpp To: sars, cullmann, vkrause, dhaumann, mwolff Cc: zetazeta, mwolff, brauch, kwrite-devel, kde-frameworks-devel, hase, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann
D17241: WIP:Disable highlighting for lines longer than 1024 characters.
sars added a comment. @zetazeta I'm not sure the highlighting length limit is worth an option as the editing of the document is not effected. I it is just a bit inconvenient that the highlighting disappears... REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D17241 To: sars, cullmann, vkrause, dhaumann, mwolff Cc: zetazeta, mwolff, brauch, kwrite-devel, kde-frameworks-devel, hase, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann
D17241: WIP:Disable highlighting for lines longer than 1024 characters.
sars marked 2 inline comments as done. sars added a comment. @dhaumann: You are right, the highlighting of the document still takes place and it is only KateRenderer that stops using the highlighting info for all lines longer than 1024 characters. All shorter lines are highlighted. So the length limit option is still the same: Wrapping long lines on opening a document, just 25 times longer ;) The disabled high-lighting **is** a workaround, but if we don't bring in some radical changes it will choke at some point anyways. So I would say that the target is to get the limits as high as possible. I saw that Visualstudio code uses a similar limit for highlighting lines, but I don't know if that disables the highlighting totally I found this workaround by running Kate in Hotspot ;) I can continue a bit to see if I can get the 1024 limit raised... REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D17241 To: sars, cullmann, vkrause, dhaumann, mwolff Cc: zetazeta, mwolff, brauch, kwrite-devel, kde-frameworks-devel, hase, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann
D17241: WIP:Disable highlighting for lines longer than 1024 characters.
sars updated this revision to Diff 47146. sars added a comment. - Use a bit different config key to force a value reset for the limit. - Fix reading the config value. - Update the line-highlight disabled info message and position. REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17241?vs=46957=47146 BRANCH disable_hl2 REVISION DETAIL https://phabricator.kde.org/D17241 AFFECTED FILES src/buffer/katetextbuffer.cpp src/document/katedocument.cpp src/render/katerenderer.cpp src/render/katerenderer.h src/utils/kateconfig.cpp To: sars, cullmann, vkrause, dhaumann, mwolff Cc: zetazeta, mwolff, brauch, kwrite-devel, kde-frameworks-devel, hase, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann
D17241: WIP:Disable highlighting for lines longer than 1024 characters.
zetazeta added a comment. Guys this is great work. Can we please make the limits configurable? Each developer will have a different sweet spot. I don't want to burden this revision, maybe we can make a bug about configuring the limits and leave that part to someone confortable with the configs. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D17241 To: sars, cullmann, vkrause, dhaumann, mwolff Cc: zetazeta, mwolff, brauch, kwrite-devel, kde-frameworks-devel, hase, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann
D17241: WIP:Disable highlighting for lines longer than 1024 characters.
dhaumann added a comment. Is it correct that highlighting in the document (i.e. KSyntaxHighlighting) still takes place for the entire line, and this change simply only changes the fact that KateRenderer stops using the highlighting info after 10 columns? Prior to accepting this patch, would it be possible to fix this differently, i.e. by profiling and fixing this properly instead of applying a workaround? INLINE COMMENTS > katetextbuffer.cpp:71 > , m_newLineAtEof(false) > -, m_lineLengthLimit(4096) > +, m_lineLengthLimit(10) > , m_alwaysUseKAuthForSave(alwaysUseKAuth) Hm, isn't the line length limit configurable in kateconfig.cpp? There you will find a key called const char KEY_LINE_LENGTH_LIMIT[] = "Line Length Limit"; that is used to read/write the config file. I think if this is changed here to 10, it should be changed to the same value in KateDocumentConfig::readValue(): setLineLengthLimit(config.readEntry(KEY_LINE_LENGTH_LIMIT, 4096)); And I would even suggest to rename as follows: - const char KEY_LINE_LENGTH_LIMIT[] = "Line Length Limit"; + const char KEY_LINE_LENGTH_LIMIT[] = "Line Length Limitation"; Since then, all users will automatically get a reset here. Without this change, all users will stay on the 4096 limit. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D17241 To: sars, cullmann, vkrause, dhaumann, mwolff Cc: mwolff, brauch, kwrite-devel, kde-frameworks-devel, hase, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann
D17241: WIP:Disable highlighting for lines longer than 1024 characters.
sars added a comment. The highlighting limit is now returned in a function in KateRenderer as it is used also in katedocument.cpp for the warning/information message. INLINE COMMENTS > mwolff wrote in katerenderer.cpp:400 > this style-change should be submitted independently of this code review I figured that since I'm indenting the whole section and touching the line anyways, that fixing the style does not hurt. ;) (Phabricator is not showing the white-space change) REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D17241 To: sars, cullmann, vkrause, dhaumann, mwolff Cc: mwolff, brauch, kwrite-devel, kde-frameworks-devel, hase, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann
D17241: WIP:Disable highlighting for lines longer than 1024 characters.
sars updated this revision to Diff 46957. sars edited the summary of this revision. sars added a comment. Add a message to inform about why the lines are not highlighted. Add a note about disabled highlighting to the wrapped lines warning. Increase the default line length limit to 100 000 characters per line. REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17241?vs=46633=46957 BRANCH disable_hl2 REVISION DETAIL https://phabricator.kde.org/D17241 AFFECTED FILES src/buffer/katetextbuffer.cpp src/document/katedocument.cpp src/render/katerenderer.cpp src/render/katerenderer.h To: sars, cullmann, vkrause, dhaumann, mwolff Cc: mwolff, brauch, kwrite-devel, kde-frameworks-devel, hase, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann
D17241: WIP:Disable highlighting for lines longer than 1024 characters.
sars retitled this revision from "Disable highlighting for lines longer than 1024 characters." to "WIP:Disable highlighting for lines longer than 1024 characters.". REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D17241 To: sars, cullmann, vkrause, dhaumann, mwolff Cc: mwolff, brauch, kwrite-devel, kde-frameworks-devel, hase, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann
