After renaming a file and then selecting another file immediately the just selected file stays selected.
BUG: 388555
ngraham | |
michaelh | |
elvisangelaccio |
Dolphin |
After renaming a file and then selecting another file immediately the just selected file stays selected.
BUG: 388555
Steps to reproduce:
$ mkdir /tmp/test
$ cd /tmp/test
$ touch a.tmp b.tmp
$ dolphin /tmp/test
In dolphin:
Expected result:
Automatic diff as part of commit; lint not applicable. |
Automatic diff as part of commit; unit tests not applicable. |
src/views/dolphinview.cpp | ||
---|---|---|
1362 | I'm not 100% convinced. Why does the model return a wrong index? If I select b.tmp, here I'd expect the index for b.tmp, not the one for a.tmp |
@akrutzler You are right that this is a regression after 478f404b8a.
@emateli Can you have a look?
Unfortunately I do not have as much time as I would like to investigate this, however I suspect it may have to do with slotRenamingFinished and forceUrlsSelection. As in the slot is emitted after the files(s) have finished renaming but at that point the user has already changed the selection. If that's the case it may make a bit more sense to handle this inside the forceUrlSelection method instead.
Hm I could not find a way to solve this inside the forceUrlSelection method. But maybe I am missing something.
src/views/dolphinview.cpp | ||
---|---|---|
1362 | m_currentItemUrl is not set when a file is selected. I tried to set it when the selection changes, but that does not work. Therefore, at this point, m_currentItemUrl stores the name of the file you just renamed. |
Does not work for me.
For testing:
select a.tmp <F2> type aaa
Do not hit <Enter>
Select b.tmp with mouse - No need for speed :-)
src/views/dolphinview.cpp | ||
---|---|---|
1597 | Observation: |
Maybe our dolphins differ. With fresh compile from origin/Applications/17.12 this is unexpected
I usually test against the latest master. Did you apply my patch after checking out origin/Applications/17.12? The easiest way to do this is using arc, a command-line interface to Phabricator. To apply this particular patch use arc patch D9711.
@ngraham are you able to reproduce this behavior?
Just rechecked it. It does indeed work as expected. I'm very sorry, I must have muddled something up.
I had a look and this bug is caused by the forceUrlsSelection() call in slotRoleEditingFinished() (added by 478f404b8a).
The problem is that we are marking the item being renamed as the m_currentItemUrl, but that is not necessarily correct as this bug shows.
@emateli is right that forceUrlsSelection() is a better place to put this check in. Adding
if (m_container->controller()->selectionManager()->hasSelection()) { return; }
at the beginning of the method fixes the bug for me.
With that, D6312 doesn't work anymore. Because when calling forceUrlsSelection() in slotRoleEditingFinished(), there is always a selection.
src/views/dolphinview.cpp | ||
---|---|---|
1597 | Ah right, of course. Well, let's go with this patch then. But please push to master only, I'm not confident that we won't break something else :/ |