ListPanel: don't go back in history if refresh failed
ClosedPublic

Authored by janlepper on Jan 15 2017, 7:21 PM.

Details

Summary

This changes the refresh behavior of the list panel to stay at the URL which failed to refresh, instead of going back in the panel's history until the refresh is successful.

I personally prefer this behavior - what are your opinions?

One benefit is that asynchronous refresh would be more easy to implement.

I intentionally left the indentation for now, to make the diff more readable.

Diff Detail

Repository
R167 Krusader
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
janlepper updated this revision to Diff 10193.Jan 15 2017, 7:21 PM
janlepper retitled this revision from to ListPanel: don't go back in history if refresh failed.
janlepper updated this object.
janlepper edited the test plan for this revision. (Show Details)
janlepper set the repository for this revision to R167 Krusader.
janlepper updated this object.Jan 15 2017, 7:52 PM
janlepper added a project: Krusader.
janlepper added a reviewer: Krusader.
janlepper added a subscriber: Krusader.
martinkostolny accepted this revision.Jan 15 2017, 8:40 PM
martinkostolny added a reviewer: martinkostolny.
martinkostolny added a subscriber: martinkostolny.

I'm definitely for this change! I don't like loosing the path when e.g. external disk is temporarily removed. Thanks!

This revision is now accepted and ready to land.Jan 15 2017, 8:40 PM
janlepper updated this object.Jan 15 2017, 8:46 PM
janlepper edited edge metadata.
This revision now requires review to proceed.Feb 6 2017, 8:27 AM
janlepper added a comment.EditedFeb 6 2017, 8:28 AM

Before I go forward I would like some more opinions ;)

Btw: Am I right assuming that having "Krusader" as a blocking reviewer requires all members to accept?

asensi accepted this revision.Feb 8 2017, 9:06 AM
asensi added a reviewer: asensi.
asensi added a subscriber: asensi.

This changes the refresh behavior of the list panel to stay at the URL which failed to refresh, instead of going back in
the panel's history until the refresh is successful.

I personally prefer this behavior - what are your opinions?

One benefit is that asynchronous refresh would be more easy to implement.

It looks good to me.

Btw: Am I right assuming that having "Krusader" as a blocking reviewer requires all members to accept?

I'm going to accept it, so everybody will see how that works :-)

This revision is now accepted and ready to land.Feb 8 2017, 9:06 AM
janlepper edited reviewers, added: abika, gengisdave, yurchor; removed: Krusader.Feb 8 2017, 1:25 PM
This revision now requires review to proceed.Feb 8 2017, 1:25 PM
In D4146#84030, @asensi wrote:

Btw: Am I right assuming that having "Krusader" as a blocking reviewer requires all members to accept?

I'm going to accept it, so everybody will see how that works :-)

Apparently it doesn't - so I added the rest of the members as blocking reviewers.

yurchor accepted this revision.Feb 8 2017, 1:39 PM
yurchor edited edge metadata.
martinkostolny accepted this revision.Feb 8 2017, 1:45 PM
martinkostolny added a reviewer: martinkostolny.
abika accepted this revision.Feb 8 2017, 3:20 PM
abika edited edge metadata.

I personally prefer the old behavior. After restart going back to the "nearest" parent folder of a deleted path was pretty nice. And if it was a removable device I can mount it and go back to the folder with the history.

But whatever, another user setting for this would be too much. And yes, the while (true) loop wasn't nice.

In D4146#84250, @abika wrote:

I personally prefer the old behavior. After restart going back to the "nearest" parent folder of a deleted path was pretty nice. And if it was a removable device I can mount it and go back to the folder with the history.

But whatever, another user setting for this would be too much. And yes, the while (true) loop wasn't nice.

Well you can use the history to go to the nearest refreshable parent folder - almost as convenient ;)

In D4146#91556, @abika wrote:

Oh - well I guess we can close this afterwards ^^

I assume gengisdave won't comment on this (hasn't been active for a while)

This revision is now accepted and ready to land.Mar 3 2017, 2:31 PM
aacid requested changes to this revision.Mar 3 2017, 8:37 PM
aacid added a subscriber: aacid.

Patch doesn't seem to apply

This revision now requires changes to proceed.Mar 3 2017, 8:37 PM
janlepper updated this revision to Diff 12192.Mar 5 2017, 6:04 PM
janlepper edited edge metadata.

rebased

aacid resigned from this revision.Mar 5 2017, 6:20 PM

Krusader people, will someone please commit this?

This revision is now accepted and ready to land.Mar 5 2017, 6:20 PM
This revision was automatically updated to reflect the committed changes.