Backup on save: Support time and date string replacements
ClosedPublic

Authored by dhaumann on Sep 10 2019, 1:29 PM.

Details

Summary

Supported variables are:

Screenshot:

BUG: 403583
FIXED-IN: 5.63

Diff Detail

Repository
R39 KTextEditor
Branch
bug
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 16377
Build 16395: arc lint + arc unit
dhaumann created this revision.Sep 10 2019, 1:29 PM
Restricted Application added projects: Kate, Frameworks. · View Herald TranscriptSep 10 2019, 1:29 PM
Restricted Application added subscribers: kde-frameworks-devel, kwrite-devel. · View Herald Transcript
dhaumann requested review of this revision.Sep 10 2019, 1:29 PM

KateDialogs contains the following:

if (uiadv->edtBackupSuffix->text().isEmpty() && uiadv->edtBackupPrefix->text().isEmpty()) {
    KMessageBox::information(
        this,
        i18n("You did not provide a backup suffix or prefix. Using default suffix: '~'"),
        i18n("No Backup Suffix or Prefix")
    );
    uiadv->edtBackupSuffix->setText(QStringLiteral("~"));
}

Theoretically, if a variable exands to nothing (JS:..., ENV:...) we could still run into both empty prefix and suffix in katedocument.cpp.

This revision was not accepted when it landed; it landed in state Needs Review.Sep 10 2019, 1:32 PM
This revision was automatically updated to reflect the committed changes.
dhaumann reopened this revision.Sep 10 2019, 1:35 PM

arc bug: it's not yet committed.

dhaumann updated this revision to Diff 65754.Sep 10 2019, 1:37 PM
  • rebase to master
dhaumann edited the summary of this revision. (Show Details)Sep 10 2019, 1:43 PM
dhaumann added a subscriber: ngraham.

@ngraham: This may be interesting to you as well (and a prereq for the External Tools plugin)

dhaumann retitled this revision from Variable expansion: Prefer return value over return argument to Backup on save: Support time and date string replacements.Sep 10 2019, 3:23 PM
dhaumann edited the summary of this revision. (Show Details)
cullmann accepted this revision.Sep 11 2019, 5:05 AM

Looks fine.

This revision is now accepted and ready to land.Sep 11 2019, 5:05 AM
dhaumann added inline comments.Sep 11 2019, 6:09 AM
src/document/katedocument.cpp
2594 ↗(On Diff #65754)

Shall we wrap all this with

if (!(backupSuffix.isEmpty() && backupPrefix.isEmpty())) {...}
cullmann added inline comments.Sep 11 2019, 7:18 AM
src/document/katedocument.cpp
2594 ↗(On Diff #65754)

Why not, saves useless evals.

dhaumann added inline comments.Sep 11 2019, 8:47 AM
src/document/katedocument.cpp
2594 ↗(On Diff #65754)

No: that's exactly the point: I currently don't think it's possible, but both evals could result in empty strings.

Can you do the integration? Author does not matter.

dhaumann updated this revision to Diff 65843.Sep 11 2019, 12:11 PM
  • Avoid running the backup when no backup prefix/suffix was provided
This revision was automatically updated to reflect the committed changes.
dhaumann marked 3 inline comments as done.