Prompt user to save or discard changes
ClosedPublic

Authored by shubham on Jul 15 2019, 6:01 PM.

Diff Detail

Repository
R40 Kate
Branch
config
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 13984
Build 14002: arc lint + arc unit
shubham created this revision.Jul 15 2019, 6:01 PM
Restricted Application added a project: Kate. · View Herald TranscriptJul 15 2019, 6:01 PM
Restricted Application added a subscriber: kwrite-devel. · View Herald Transcript
shubham requested review of this revision.Jul 15 2019, 6:01 PM

Actually I am not sure if we want this behavior at all.
Most config dialogs don't do this.

shubham added a comment.EditedJul 15 2019, 6:07 PM

It is done in dolphin.

Since when? For me Dolphin 19.04 doesn't ask.

dolphin 19.07.70
Maybe because I am on KDE neon dev unstable edition

Ok, yes, you are right. And I assume the code is copied from there, or? Which is actually license incompatible, as the code is ATM LGPL v2+, not GPL.
For the actual implementation: m_unsavedChange seems to be not needed, as we have m_dataChanged, or?

shubham updated this revision to Diff 61822.EditedJul 15 2019, 6:53 PM

Remove m_unsavedChange.
Yes, this is copied from dolphin

+1 for fixing the bug, but I'm left wondering if it makes sense to require every settings window to reinvent the wheel like this. Couldn't we add this kind of logic globally into the KPageDialog, maybe?

That way if we ever decide that these dialogs are annoying, we can turn them off in one place rather than having each app wind up with inconsistent behavior.

I would be ok to merge this, thought I don't know if the license is ok, given that is a 1:1 copy from the other program, or?

It is exact copy of the dolphin code.

Then the license doesn't match, if I don't misread the license headers of the file you copied it from and ours.

shubham added a comment.EditedJul 25 2019, 2:10 PM

I see even though wording are not same, they both mean the same thing. ie.(GNU GPL)?So what to do?

You could ask the original author if LGPLv2+ would be ok.

The license in dolphin says "you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation; either version 2 of the License, or (at your option) any later version".

either version 2 of the License, or (at your option) any later version

I think this means v2+.

I will try asking Peter Penz, however I doubt he is still active.

Our dialog is "GNU Library General Public".
And I assume that code is new, as in the 19.04 version I had dolphin didn't ask ;=)

dhaumann added inline comments.Jul 26 2019, 4:28 PM
kate/kateconfigdialog.cpp
427

Typo: have have

435

I'd prefer to repeat event->accept + break here, instead of using fallthrough.

cullmann accepted this revision.Aug 13 2019, 8:54 PM

Given the code is trivial (a message box with two sentences) I assume we can merge this disregarding gpl/lgpl.

This revision is now accepted and ready to land.Aug 13 2019, 8:54 PM
This revision was automatically updated to reflect the committed changes.