- User Since
- Apr 20 2015, 7:20 AM (173 w, 3 d)
I think one more iteration, and this can be merged. Can you look into this again?
Wed, Aug 15
I think this goes into the right direction :-)
True... Is this a performance issue? ;)
Albeit a bi hacky, if that helps Kate users on Gnome, I am fine with this.
@devillemereuil That is good to hear. If you can, feel free to paste a MIT-licensed snippet of R-code with a function, a loop, a if-condition etc. Maybe we find the time to turn this into a unit test. The code does not need to run, it just needs to be syntactically correct.
Just to make everyone aware of what this implies:
- Every application now has a separate KTextEditor configuration
- Every application now has line numbers on by default (that includes Kile)
Tue, Aug 14
Ok from my side, although it still *feels* a bit messy :-)
Fixed in D14847 in a different way.
- Optimize by checking isEmpty()
I just tested again with KWrite - I think it behaves good there.
Hm, can you send a patch that fixes this? :-)
I was just about to merge this patch. But the crash still happens for me also with this patch - so it's still not fixed.
I guess ok - if issues arise, we have to fix them :-)
Sorry for the delay. Shall we take care of this?
Looks good. But along with this change, please change the docbook in kate.git:
Thanks, Martin just change the kateversion number, which was changed again later by us again. So I will proceed now. Thanks.
For those who are curious: This change is required since modelines.xml is a file that is included in many other xml files via IncludeRules. Given our policy of new highlighting files should be MIT licensed, this also applies to included definitions. That's why this relicensing is being done for this file. Your support is very much appreciated here.
Mon, Aug 13
We now have application-local settings. What now? Are we closer to solving this? ...I guess not: Either it's on for all applications or off for all applications. The user still needs to set this up. @cullmann and @ngraham Any comments? ;)
...which raises the question of the minimum required Frameworks version needed by the KDE Application release.
I guess it's better to accept this now than later to have a longer testing period at Akademy and until the frameworks release.
- Add another test, now all code paths are tested
Ok - then with minor changes, let's move this forward. Who is going to do the work? :-)
- Apply proposed fixes
- Merge branch 'master' into CommandScript
@kossebau Akademy is in full swing, and we try to get the review requests down. Could you move this forward? Would be really nice :-)
What is the current state of this? The new private functions should move to the pimpl class behind the d-pointer.
Tested again, certainly an improvement. If we find more issues, we can still improve.
Thanks goes to do - and sorry for the delay on our side.
Thanks, changes look, please commit (after running make test) :-)
Shall we integrate this, or can you commit yourself?
Ok, perfect, thanks! I think "Apache License 2.0" is fine as well. It's just important that we do know the license, because otherwise it's a mess over time.
Just curious: The test file looks good to me, but it looks to me that it also contains some redundancy: Think of it as follows: We use this for unit testing. As such, the best case is that a test file tests all the corner cases, but ideally only once. If a test fails, then we get a diff with the changed highlighting information. And if the file is 400 lines of code, checking for regressions is much more work compared to if the file only contains e.g. 50 lines. So: would it be possible to strip it down a bit? And also: Is this file freely usable, e.g. licensed under MIT? Would be nice to add a license comment in the first line.
@ngraham If you blog about it, feel free to explicitly mention that this is hopefuly very useful for the LaTeX community using Kile.
For me this works. It would be nice to get more feedback, but to be honest, I think we only get this feedback by integrating this patch and let the users test it. It's certainly not broken and fixes a usability issue we've had for a log time.
Sidenote: Looking into the code in KTextEditor, there is:
Nice patch. Please add a comment:
// Temporarily remove source model to avoid triggering unnecessary internal updates all the time
Sun, Aug 12
@vkrause We simply decided to commit this now. But a review from your side is still appreciated ;)
Could you provide a test snippet now? I will integrate this then.
Not yet thoroughly reviewed, but some thoughts:
- There is a bug report for this: https://bugs.kde.org/show_bug.cgi?id=256561 Could you verify that the issues in the bug report are addressed by this patch? See also the comments by @ngraham in this bug report.
- could you add: BUG: 256561
- Since the commit logs now always appear in the KF5 changelog, maybe the subject could read: Fix: Scroll view lines instead of not real lines for wheel and touchpad scrolling
- Compile ;)
Sat, Aug 11
- Add support for getters of invalid Definitions