- User Since
- Jun 3 2015, 8:56 AM (194 w, 1 d)
Wed, Feb 20
Tue, Feb 12
The idea is sane :)
Can you also return false in setOptVal() in case the scanning is ongoing? (and the corresponding note in the doxygen comments)
Mon, Feb 11
The idea is good. Could we have setOptVals() return -1 and setOptVal() false if the scanning is ongoing and the delayed setting of the value implemented in Skanlite?
OK this can go in.
Sun, Feb 10
Just these minor adjustments and we are there.
Wed, Feb 6
Mon, Feb 4
Tue, Jan 29
Sorry, busy month... new project at work and got a new computer to configure....
Jan 22 2019
I think the selection if we save through the PNG 16bit/color or QImage version should be done in Skanlite.cpp not in savePng().
Jan 16 2019
Jan 15 2019
- Yes the "more option" button is a bit far away. At some point it actually was at the right of all the options. That is a real possibility
Sorry for the late response. A lot of real life activities....
Jan 13 2019
My personal taste would be to only have a small tool-tip saying: See the "Add.." context menu entry for regular expression help.
- These buttons are different than the other buttons in that they modify the UI of the toolview but not the search. So mixing them is maybe not the best? One could think about moving them to the right side of the other buttons and have some type of separator between...
I would be OK with the addition of F6 and Shift+F6 to the Search plugin, but I think it would be good to have a plan for all the shortcuts and update all at once....
I'm not entirely sure removing the feature of having the lookup word in the menu is an improvement, but I just noticed that the feature is broken and only works when a word is selected.
Jan 9 2019
I'm OK with this repaint() and if you want I'm also OK with having the processEvents() until we move the actual saving to a separate thread.
Jan 8 2019
I really like the idea! :)
But this all is to just re-paint one time before we freeze for the time it takes to save of the file? Would it be better in the long run to think about moving the saving to a thread?
Jan 7 2019
I'm OK with parent->repaint()
I have not had the time to review this properly sorry :(
Besides the processEvents() I'm OK with this! :)
Jan 4 2019
Adding as a warning message in the view could be another review and needs comments from others first
Hmm... this is only done for local paths. If we only support this feature for local paths it would be much simpler to just use QDir::mkpath(), but it would be nice if it worked for remote folders too...
Jan 3 2019
@dhaumann OK the limit is too low for Kile that is clear. Visual Studio Code is limiting the highlighting on a line to 10000 characters.
Dec 31 2018
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 the beginning of the line. The highlighting makes it a bit slower, so I decreased the limit to 512.
Disable highlighting after [limit] characters on a line in stead of the whole line.
Dec 28 2018
Dec 23 2018
I think this is fine, but let Christoph and Dominik decide if we want style fixing commits.
Dec 20 2018
Dec 19 2018
@shubham I'm wondering about the original issue that you try to fix with this review request. Why do we need a dialog that warns that there are multiple documents open in Kate when we close it?
Sorry Christmastime is a bit busy...
Dec 16 2018
Dec 15 2018
Changing the next/previous slots to go to the items with line numbers would be a good change.
I'm not sure why you have to move the setFocus() call to the beginning of the function... your comment says something about selection...
I think one magic build name is not the best way to enable auto-hiding.
Dec 10 2018
Sorry, I was a bit busy when this review came and then I forgot about it :(
Dec 9 2018
Dec 8 2018
Wouldn't it be better to make it possible to configure the current CppCheck in stead of adding a new one?
@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...
@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.
- 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.
Dec 7 2018
There is not much else we can do than just have a dummy like this, with the current state of the twain wrapper.
Sorry! This problem has totally passed me by before this... I'll have a look.
Dec 6 2018
The highlighting limit is now returned in a function in KateRenderer as it is used also in katedocument.cpp for the warning/information message.
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.
Now I committed to master. Should I back-ported it to 18.12?
I guess there are no objections to commit this :)
OK, I have used this patch actively for a week now, and I have not noticed any regression... ;)
Remove unneeded temporary
Dec 4 2018
I agree, we need to do something about the line length limit (that only wraps the lines at the limit when opening the file).
Dec 3 2018
It is maybe not optimal to "hide" a bug fix among a bunch of code cosmetics, but the fix is good :)
Dec 2 2018
What if you have Project, CTags and the new Zeal plugins active? You need to differentiate them somehow.
Let's take that one :)
I think the text could be "Cannot run command: %1\nWork path %2 does not exist."
I think this is a fix to the bug 347311
Dec 1 2018
(Remove the unrelated change in src/view/kateview.cpp)
Also disable highlighting in selections for lines that are longe than 1024
Nov 29 2018
Note: this is especially bad when the "on the fly spellchecking" is enabled as visibleRange() is used to "optimize" and only highlight what is currently visible. It is called multiple times.
Use EditingTransaction in stead of just disabling highlighting
If we get it working properly we could make the limit much higher. I tried with 30000 characters and it still worked very good. (except when selecting the line)
I tried it and it improves two things first the multiple replace is much faster and second you get the whole replace operation in one undo. I'll do an update when I have cleaned up the experiments a bit :)
Nov 12 2018
Nov 11 2018
Nov 10 2018
The new connections are better because you get an error just for these problems. Explicitly failing is better than silently doing so on run-time.
Adding a function is not breaking backwards compatibility. You can still use Skanlite compiled against an older version with this new version.
Nov 9 2018
I'm a bit confused... I thought KDevelop had much more advanced plugins/features for this? What does KDevelop need this old plugin for? Non-C/C++ languages or what?
Nov 4 2018
I would like to give a -1 for removing the plugin I think that the feature is really something that has potential to be great!
Nov 3 2018