Differential D18116

Add TODO KF6 note
ClosedPublic
Actions

Authored by loh.tar on Jan 8 2019, 7:41 PM.

Details

Reviewers
cullmann
dhaumann
Group Reviewers
KTextEditor
Commits
R39:574b08d08111: Add TODO KF6 note
Summary

Since there are fancy "in view" messages which show all loading trouble,
is there no need to fetch and show some error message.

See also https://phabricator.kde.org/D18117

Diff Detail

Repository
R39 KTextEditor
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
loh.tar created this revision.Jan 8 2019, 7:41 PM
Restricted Application added projects: Kate, Frameworks. · View Herald TranscriptJan 8 2019, 7:41 PM
Restricted Application added subscribers: kde-frameworks-devel, kwrite-devel. · View Herald Transcript
loh.tar requested review of this revision.Jan 8 2019, 7:41 PM
loh.tar added a comment.Jan 8 2019, 7:45 PM

I have it only poor tested with our big S&R file, so no test was made with other trouble causing stuff.
This need a patch in Kate, will try to link that

KateBuffer::openFile

  • There is no distingues from
    1. "create new file by command line" and
    2. "restore files of session"
  • When..
    1. is the error wrong
    2. makes to me "new file" not much sense

Is there a way to check wat is going on, 1) or 2) ? Guess, yes, and you can point me

loh.tar updated this revision to Diff 50027.Jan 21 2019, 9:34 PM
  • Don't set m_openingErrorMessage in KateBuffer::openFile
  • m_openingErrorMessage is now nowhere set
dhaumann added a comment.Jan 21 2019, 10:11 PM

Just for info: If openingErrorMessage is indeed not set anymore, this is a behavior change, since KTextEditor::Document::openingErrorMessage() is public API. Looking at lxr.kde.org, it seems this is not used (but ->openingError() is used, see https://lxr.kde.org/source/kde/applications/kate/addons/filetree/katefiletreemodel.cpp#0554).

cullmann added a comment.Jan 22 2019, 12:14 PM

I would not change that here, I would just ignore it in the application, as said in the other diff.

cullmann requested changes to this revision.Feb 2 2019, 3:22 PM

Just to be sure:

We can apply D18117 and don't need to change here anything or?

This revision now requires changes to proceed.Feb 2 2019, 3:22 PM
loh.tar updated this revision to Diff 50759.Feb 3 2019, 7:50 AM
loh.tar retitled this revision from Don't show document load trouble twice to Add TODO KF6 note .
loh.tar edited the summary of this revision. (Show Details)
loh.tar set the repository for this revision to R39 KTextEditor.
  • Revert all
  • Add TODO KF6 note to header
  • Update Summary
loh.tar added a comment.Feb 3 2019, 7:53 AM
In D18116#403838, @cullmann wrote:

Just to be sure:

We can apply D18117 and don't need to change here anything or?

It seems so :-)

dhaumann added inline comments.Feb 4 2019, 5:45 PM
src/include/ktexteditor/document.h
394

Hm, is it really that bool openingError() is not needed anymore? I thought this is about bool openingErrorMessage() that is not required anymore.

loh.tar updated this revision to Diff 50875.Feb 4 2019, 7:14 PM
  • Move to correct function

Good catch :-)

dhaumann accepted this revision.Feb 4 2019, 8:33 PM
This revision was not accepted when it landed; it landed in state Needs Review.Feb 5 2019, 12:31 PM
Closed by commit R39:574b08d08111: Add TODO KF6 note (authored by dhaumann). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Msrc/include/ktexteditor/document.h (2 lines)

Diff 50946

View Options

src/include/ktexteditor/document.h

Loading...