- User Since
- Aug 13 2018, 6:55 PM (14 w, 1 d)
Oct 19 2018
I don't think so. I have a KDE identity and login for the KDE bugzilla. Is one of these what you're asking about?
Made QStringList const too.
Thank you for your patience and feedback on this. I appreciate it since I'm still a bit new to C++ and this workflow. Would you like me to check the done boxes above with the in line diff comments? Or is that something the reviewer(s) do? Thanks.
Fixed memory leak. Added const to several QStrings. Update for loop to C++11 style.
Oct 18 2018
Add failsafe for blank filename in save options form. Placeholder and default file name is now %d. Also
improve regex that checks for similar file names for edge case with only sequential number file name.
I think the empty file name template setting defaulting to sequential file number is an interesting idea. It is a good fail safe. If that seems like a reasonable add on to this diff, I can add %d as the default string coming from SpectacleConfig if save-filename-format is empty.
Removed more merge conflict markers. Sorry. Updated local method call to use upstream name.
removed some merge conflict markers
Sorry. I didn't mean for this to seem to be stalled. I submitted a new patch with the result of our conversation above. It uses arc and is rebased on the upstream git repository as of today so hopefully we can avoid some of the merging issues of last. The new patch is D16304.
Oct 15 2018
merged with origin master to hopefully fix the patch
Oct 10 2018
rebased on origin master I think
Oct 6 2018
Oct 2 2018
Sep 17 2018
I like how you've rephrased the instructions. Breaking it into two lines ends up being clearer and more concise.
Sep 12 2018
What made you step and and decide to contribute to KDE?
I've always wanted to contribute to something. I was very happy with what is going on in KDE lately (Plasma in particular but also the more common apps) and Nate's blog made it seem very approachable, even for someone new to KDE/Qt/C++.
I agree about the help text on the use of this placeholder. I simplified it based on your suggestion. I think it is better but perhaps not perfect and am open to further suggestions on it.
Sep 11 2018
Am I to land this myself? I tried with arc but I got the following error:
Let's try this diff. I also tidied up what will happen for large sequential file numbers - they roll over at 1,000,000.
Sep 7 2018
Let's try this one.
Sep 5 2018
I have reverted the commit that was causing the issue on my local feature branch which I hope allows the diff to work. If it doesn't, I'll go one step further and start a new branch from the upstream repository and reapply the changes manually.
Aug 28 2018
I believe I made the updates from the review. Is uploading a new diff that includes the original and review changes the preferred next step here? Thank you. This is my first go around with this.