- User Since
- Jul 30 2015, 8:46 PM (202 w, 2 d)
- addConfigEntry(ConfigEntry(LineLengthLimit, "Line Length Limit", QString(), 4096));
+ addConfigEntry(ConfigEntry(LineLengthLimit, "Line Length Limit", QString(), 10000));
Then let's try that. Even for all cases without hitting the limit the new code is faster.
I am not sure about the warning.
The line length limit default should be increased it think, we could just try something like 10000 for a start.
I would propose we try this without a warning and see if we get negative feedback.
Normally this should only kick in situations were before we slowed down to "unusable".
Would that be ok?
Wed, Jun 12
I would say: go for it.
And thanks for taking care!
Tue, Jun 11
Not that I have the most karma here, but I agree with the reasoning, dead things are dead, zombies are porting aids at best.
For the outer vector, yes.
Actually, before one starts to touch this for such things, I would like to have the KTextEditor stuff ported to use the "themes" as provided by KSyntaxHighlighting.
Mon, Jun 10
Coding style wise, could one change:
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.
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 https://bugs.kde.org/show_bug.cgi?id=345775 you still see selection but not other stuff.
I have drafted a small limit to attributes like I proposed.
I played a bit around with the nightly builds from the binary factory.
Look good enough for me for normal use.
There could be some fine tunings like better styling of the status bars, but beside that, I saw no obvious issues compared to the Linux version.
Hmm, somehow this got stalled :/
Nice that you take care to integrate that.
Sat, Jun 8
Ok for me, is a good initial start!
Thanks for the work on this.
Perhaps we must go away from the implicit listing of the syntax files and really just add them all to the CMakeLists.txt,
Wed, Jun 5
Nice, I use ninja here, cool to have!
I think as an initial version this is ok.
Fri, May 31
Hmm, ok, anyways, this change looks ok =)
Could one adapt the theme and remove the color later?
Screenshots look better!
(the hard coded color was already there, therefore this doesn't block this request)
Looks reasonable + test case, nice.
Btw., would it be possible to remove the last remaining hard coded color <itemData name="CommentTitle" defStyleNum="dsComment" color="darkorange" bold="1"/> here, too?
Thu, May 30
Ok, seems reasonable, please push this.
Thanks for taking care of this on Windows!
Wed, May 29
Tue, May 28
I ATM have no access to my compile env. here, please commit the fix.
It would be best to have some test for this, too, for future reference.
Actually the resource should be recreated on syntax file changes and be used.
Mon, May 27
I think this is fine then, please remove the decl. of bool KTextEditor::DocumentPrivate::checkOverwrite(QUrl u, QWidget *parent), too and push this, thanks!
Change looks reasonable + test case => thanks, please push
Sun, May 26
My minimal Python skills say this looks ok.
If you have the time, some proper application object should do KWrite no harm.
At the moment we have one combobox "Whitespaces" and one checkbox "Highlight tabulators" in "Whitespace Highlighting".
Ah, you mean because of the "show tabs" extra option, yeah, forgot that :/
What would be the concrete content of the combobox then?
Thank you for providing the patch!
Helping a bit to bring it up to speed is much easier and more motivating than to start from scratch for a thing one doesn't need oneself that much.
Btw., if should push, I would need your name + email address for the git commit, thanks.
I think this look OK.
Can you commit yourself? Or shall I push for you?
I think then it is a bit inconsistent.
Would it really be that confusing, given until you write your find/replace stuff inside, you have the labels?
Could you try: KStandardShortcut::shortcut(KStandardShortcut::ActualSize)
I think this feature is nice.
Given we already have now some enum for the show spaces stuff (None/Trailing/All) I would just vote to have some "Smart" 4th variant inside.
We can later tweak this "Smart" variant like we want it to behave, actually the "always show stuff on current line and for selections" doesn't sound that bad.
Hi, this is a good start!
You are right that one needs one unique KTextEditor::Application, otherwise the semantics are bogus.
I think this patch is a good start to create one.
If you have not further time or fun to work on this, I can give it a try.
Sat, May 25
checkOverwrite decl. should be removed, too.
I think a skeleton is ok, but is quit really just close? I thought it should try to quit the app not only close one window.
Ok, then lets do this.
Just push it.
Fri, May 24
I think this is reasonable, if one enables this, it should be shown, people can disable it, if it doesn't look nice for their font (or alter the font).
Thu, May 23
Why not, better bit highlighting than none.
Hmm, I am fine with the change beside with the delete m_foldingPreview;, I think that is there to ensure the preview is directly hidden if one toggles a folding.
It will be later deleted by the leave of the bar but I don't think the earlier delete hurts.
Please keep that, if there is no strong reason to remove it and push that, thanks.
Wed, May 22
It seems nobody takes this up, shall we close it until a new proper request is submitted?
Hi, any more feedback on this?
Ok, I see, I tried it a bit more and yes, now the folding start/end tokens are highlighted on unfold.
Btw., is it intentional that the folding start/end tokens are only sometimes highlighted on folding, still?
Ok, regression taken care of in D21331
I must confess, I am a bit confused what this patch should improve.
Could you tell again which concrete issue it fixes?
And why is the delete not useful?
I think for 2 one still needs a clever way to compute the "short" path. I think the document list tries to create shortest unique "suffixes".
For 1) to be efficient one could hash all documents with "filename" => documents with that if file names changes, that would avoid that one needs to iterate always again over all documents to collect duplicates.
Given nobody proposes a working alternative, lets go with this.
Lets give this a try.
Mon, May 20
Simple tests with an 12 million lines document (COPYING.LIB concated "a lot") show compare to the current approach usable behavior.
You start to type some word not in the document, it won't free for 10 seconds, it will just search in the background like it should.
I think the largest existing issue is IMHO the multi line stuff. Some more comments could help.
could this lead to the following regression:
Just out of curiosity: Does one need the exit(func(...)) call or wouldn't a return func...(); be enough?
:=) You are right, without that, it might be even faster as we only do it once.
Sun, May 19
Looks reasonable, thanks.
I think one reason for the early out was that actually even the "isXXX" checks are very expensive, at least if I remember correctly they did show up a lot in my profiling in the past.
Perhaps one should profile this once more (and if still visible) at least skip all the isXXX checks for the isDefault... case.
Otherwise I have no issues with this.
Looks ok for me.
I like the idea of having better short document names without numbers.
The algo looks a bit like what the document list tries to do with the shortest paths differentiating two files.
I think the documents stuff ignores the changed doc names as it looks for file name changes, but that might be wrong.
Hmm, perhaps updateConfig would be better to not miss other stuff.
OK with that.
I think moving the config check some lines up will do the trick.
Yes, please push, thanks.