This revision was automatically updated to reflect the committed changes.
Closed by commit R39:81a8d573ecaf: DocumentPrivate: Add option Auto
Reload Document to View menu (authored by loh.tar).
REPOSITORY
R39 KTextEditor
CHANGES SINCE LAST UPDATE
cullmann accepted this revision.
cullmann added a comment.
The menu is now ok and the feature still works for me.
And yes, the view still jumps a bit.
:P Funny enough for me it failed during testing more because of missing file
notifications, should not have tried it on NFS.
REVISION
loh.tar added a comment.
As you see, not only the menu is changed, so I update this diff for your
approval.
> I think this works well enough to be added.
Harr, I found it now not good enough and spend some more time for this. Looks
now nicely to me. The view still jumps slightly
loh.tar updated this revision to Diff 55199.
loh.tar added a comment.
- Ensure the view jumps not back when user scrolls around
- Don't reload while user scrolls
- Fix missing connect to auto reload slot when enabled by modOnHdHandler
- Change menu
F6742373: 1554131807.png
cullmann added a comment.
Hmm, if the follow_.. stuff is removed, I think the sub-menu should go away,
too.
The view_auto_reload should just be a top-level action, menus with single
entries are strange ;)
Feel free to commit this with such a change.
REPOSITORY
R39 KTextEditor
loh.tar added a comment.
> Could the "view_auto_follow" be implemented in a second review after this
is commited?
I had remove this because it's somehow unhandy. When enabled you can't scroll
up an look at some line without to be interrupted on next update. Now it works
the smart way.
cullmann accepted this revision.
cullmann added a comment.
This revision is now accepted and ready to land.
I think this works well enough to be added.
Could the "view_auto_follow" be implemented in a second review after this is
commited?
I see there is some unresolved comment about
cullmann added a comment.
I still need to test that again ;=)
REPOSITORY
R39 KTextEditor
REVISION DETAIL
https://phabricator.kde.org/D19517
To: loh.tar, #ktexteditor, cullmann
Cc: dhaumann, cullmann, kwrite-devel, kde-frameworks-devel, #ktexteditor,
gennad, domson, michaelh, ngraham,
loh.tar updated this revision to Diff 53891.
loh.tar set the repository for this revision to R39 KTextEditor.
loh.tar added a comment.
- Use new sig/slot style
- Move/Use KToggleAction in document as suggested
- Fix not working throttle logic
- Increase delay to 3sec
- Remove unneeded
loh.tar added a comment.
> Just to be on the safe side: Does this also work, if you have two views
visible showing the same document, and then the message appears?
As said, poor tested., but what needs to be taken care? Normally these
messages works well, so no idea why now not.
>
dhaumann added a comment.
Just to be on the safe side: Does this also work, if you have two views
visible showing the same document, and then the message appears? Does it also
work, if you have two main windows showing the same document (View > New
Window)?
INLINE COMMENTS
>
loh.tar updated this revision to Diff 53613.
loh.tar added a comment.
- Add actions to View menu
- Contains some garbage lines
- Still poor tested
- One issue may/is that in case of multible views/windows the actions only
get synced after an reload event :-/
CHANGES SINCE LAST UPDATE
loh.tar added inline comments.
INLINE COMMENTS
> dhaumann wrote in katedocument.h:1086
> What about this: make this a QAction with setCheckable(true). Then, you could
> reuse this action in the KTextEditor::Message. You can put this action then
> into the menu as well.
>
> ...then again, this
dhaumann added inline comments.
INLINE COMMENTS
> katedocument.h:1086
> bool m_modOnHd = false;
> +bool m_autoReloadMode = false;
> +QTimer m_autoReloadThrottle;
What about this: make this a QAction with setCheckable(true). Then, you could
reuse this action in the
loh.tar updated this revision to Diff 53548.
loh.tar set the repository for this revision to R39 KTextEditor.
loh.tar added a comment.
- Add autoReloadThrottle
- Add some var settings/signal which looks missed, pls compare to
onModOnHdReload()
- not sure if that static single shot make
cullmann requested changes to this revision.
cullmann added a comment.
This revision now requires changes to proceed.
Perhaps in addition to the message, one should just add a menu entry in Tools
like "Read Only Mode", then the user can turn it of again.
And as said, I think one needs some
loh.tar updated this revision to Diff 53296.
loh.tar retitled this revision from "DocumentPrivate: Auto reload in read-only
mode" to "DocumentPrivate: Add option "Enable Auto Reload" to ModOnHdPrompt".
loh.tar edited the summary of this revision.
loh.tar edited the test plan for this revision.
17 matches
Mail list logo
