- User Since
- Jan 25 2016, 10:25 AM (129 w, 3 d)
Sat, Jun 23
Looks reasonable to me. Can you submit the patch yourself or should I do it?
No, this looks good to me. Thanks!
Of course, nobody is exactly sitting here thinking "eeh, now I've been waiting for that guy for *weeks* to get his patch in" ;)
If you need advice, feel free to ask.
Thu, Jun 21
No, I think you should introduce a way to get the session name before the colon is added. You can introduce a new role for the item model for example, have a look at QStandardItem::setData.
Try opening a few projects in the session. The names of these projects should follow the colon afterwards.
AFAIK that is followed by the names of the projects open in that session, and that is also where your approach with chopping of the chars will probably go wrong.
KDE only uses github as a read-only mirror. You will get a notification when it is submitted.
Otherwise looks good, I can submit this later.
Wed, Jun 20
Jun 19 2018
Ah, cool. In that case I would need your email address to set you as the author of the change.
Jun 18 2018
Looks good to me, thanks! Can you submit it yourself?
Jun 14 2018
Yes sorry, I felt too tired yesterday to correctly resolve the version number merge conflict ;) Done now.
Jun 13 2018
Looks like it was already fixed for Qt 5.11.1, e15fc26e9fdbff141890a3e2e8dc4ef935d022a0 sounds a lot like it:
I'll dig around a bit and see if I can find the change in Qt causing this. In the meantime, do you want this in applications/18.04 or master only?
Jun 7 2018
May 17 2018
May 9 2018
What happens if the overload selection window is open in addition (like in KDevelop)?
May 7 2018
May 3 2018
Oh, heh, yes it does. It's just the docstring which says otherwise ("is an interface for the View"), and that's what I looked at at that time. That should be changed. ;)
I think fixing the selection rendering issue would be nice.
May 2 2018
Looks good from the implementation too so far. One thing I do not see is any changes to the cursorToX / xToCursor functions, is there really no change required there?
Awesome idea! Do you have a screenshot of how it looks?
Apr 23 2018
Didn't do super much C++ since but didn't notice anything. Thank you!
Apr 18 2018
Given the amount of problems we had with this in the past, I think this change makes perfect sense. I applied the patch here, I'll see if anything breaks in the next few days ...
I think the change makes sense, I applied the patch here, let's see how it goes.
Apr 9 2018
Oh and, you do not need to inherit QObject to use connect; you can connect to a lambda calling the member function AFAIK or so. Just omit the third argument in connect(). What you lose by doing this is the automatic disconnect of the connection when the receiver object is deleted, so make sure that doesn't happen.
Re. binary compatibility: should be fine because this class is not exported (no KTEXTEDITOR_EXPORT macro).
Apr 7 2018
Apr 4 2018
I remember seeing this odd behaviour as well. I agree, the changed version makes more sense.
Thank you for fixing this! 5.2 sounds ok to me, but let's not put it into the release we were planning for the next few days (i.e. not in 5.2.2), or should we?
Apr 3 2018
Apr 1 2018
For KDevelop this is fine, I don't think we have any objection.
Mar 30 2018
Change looks good (the previous code definitely looks like nonsense), but what does this mean for existing settings, saved previously?
Mar 29 2018
Mar 19 2018
Makes sense for me, I'm not aware of any common global shortcut using Alt+F9. Do you have commit access?
Uh, good find. Thanks for the patch. I should somehow have used a mutex locker for that ...
Mar 13 2018
Looks good to me, thanks!
Mar 5 2018
Looks godd, thank you!
Feb 25 2018
I looked it up, it depends on the file system. NTFS always uses UTF8, but FAT uses some weird 1980's charset. So I think this breaks if you open files from FAT file systems.
Feb 24 2018
Looks good to me, also because null termination sounds better than \n (filenames can easily contain \n although they usually don't).
Jan 13 2018
Jan 2 2018
I'll add some people who have recently been working on kdev-php.
Dec 30 2017
Heh. Fix looks obviously correct to me (good find), and tests are always nice.
Dec 24 2017
Dec 23 2017
After this was submitted master doesn't compile for me, and if I fix the compile in the trivial way the test fails. Can you have another look?
All the changes look sensible to me, thanks!
Dec 21 2017
All three changes look good to me, although the formatting of comments is of course only useful until that clang bug is fixed ... but still, better than nothing and esp. when we already have the implentation in the codebase anyways.
Dec 17 2017
String change is probably ok if there is enough time before the next release (a few weeks) AFAIU. My two cents about where to merge it would be, put it in 5.2 only if you consider it a bug fix -- i.e. if there are projects which do not parse properly because this feature is missing go for it, otherwise put it in master. There's nothing worse than adding regression bugs in patch releases because of minor features like this.
I'm not up-to-date with the PHP standard, but guessing what the language feature does, code-wise this looks fine to me.
Dec 14 2017
Not much is left of the original patch but this change makes sense to me ;) thanks!
Dec 13 2017
Yeah, sorry, I'm also against this. Linking an extra lib we depend on anyways is a problem a compuer has to deal with, extra code is a problem humans have to deal with. The former wins against the latter unless there is a very good reason why not.
Dec 12 2017
Hmm. There's a comment in the line above which states it explicitly uses canonicalFilePath to avoid issues with symlinks. If we resolve symlinks like you suggest, there will be situations like files which are part of a project but for which the project's root directory is not a prefix of the file path. Are we somewhat sure this doesn't break in other places?
Dec 10 2017
Dec 7 2017
Is this actually faster? Why?
Dec 4 2017
Hmm, this will break build of all the plugins, no? Other than that, I'm in favour of this change, thanks for the work!
Dec 3 2017
Dec 1 2017
Nov 30 2017
Yes, cool feature, go for it!
Nov 29 2017
Thanks! I'll submit this as well.
Ah, you don't have write access to KDE repos? Then I will submit this for you. Thanks!
Alright, if you think it makes sense, submit it. It's certainly an improvement over the old situation. Thanks!
Nov 28 2017
Hmpf, right, QList compares its head to its tail which is certainly not atomic. I *think* for QVector this would be safe (it compares its size to zero), but better not take chances. Sorry that this is such a hassle.
Ok, but can we not at least set the state to Ended when the connection breaks, or something like that?
Hm yes, ok, so it makes sense why this fixes the crash. I think we can keep this as a safety guard. However I think the actual problem is that the state is not set to EndedState when the debug session actually ends, which also has other undesirable consequences (e.g. KDevelop doesn't switch back to code view, etc.). Do you have an idea why this happens?
To clarify my comment above, if what I said is correct, I would suggest to call the function "processFailedToStart"or so, and make it a non-slot.
I think I hit this issue yesterday when testing kdev-xdebug, esp. the debugger wouldn't stop properly when stepping over the end of the program. I don't quite understand what issue exactly this change fixes and why, can you write a few lines?
Very nice, thanks.