Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix sort order after file operations #2260

Merged
merged 7 commits into from
Aug 6, 2023
Merged

Fix sort order after file operations #2260

merged 7 commits into from
Aug 6, 2023

Conversation

jeremypw
Copy link
Collaborator

@jeremypw jeremypw commented Jul 20, 2023

Fixes #2179
Fixes #2271
Fixes #2288

  • Fix resorting after adding/renaming a file
  • Fix appearance/persistence of Code backup files if "Show Hidden Files" is "OFF"
  • Added insert_sorted function to model
  • Added throttled resort function to model
  • Used appropriate model functions
  • Fixed side effects on rename after new file creation
  • Made is_hidden property construct-only and include files with ~ suffix.

* Add insert_sorted function to model
* Add throttled resort function to model
* Use appropriate model functions
* Fix side effects on rename after new file creation
@jeremypw jeremypw changed the title Fix auto-update view after file operations Fix sort order after file operations Jul 20, 2023
@jeremypw jeremypw marked this pull request as ready for review August 3, 2023 09:11
@jeremypw jeremypw requested a review from a team August 3, 2023 09:11
zeebok
zeebok previously approved these changes Aug 5, 2023
Copy link
Contributor

@zeebok zeebok left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. I will give it a test but feel free to merge anyway

@zeebok
Copy link
Contributor

zeebok commented Aug 5, 2023

Did some testing. I tried by editing the readme, closing, re-opening to edit, closing, etc. Usually worked but occasionally it would not sort. Not sure if a different issue is showing itself here or is related to this.

After editing README:
Screenshot from 2023-08-05 14 11 57

After closing README:
Screenshot from 2023-08-05 14 12 23

Additionally the temp files don't disappear until I refresh with <Ctrl R>

@zeebok zeebok self-requested a review August 5, 2023 18:19
@jeremypw jeremypw dismissed zeebok’s stale review August 5, 2023 18:31

Regression found

@jeremypw
Copy link
Collaborator Author

jeremypw commented Aug 5, 2023

@zeebok Yes, I just noticed that myself. It works if you delete a file from the terminal but for some reason not if it is a backup file deleted by Code. Investigating....

@jeremypw
Copy link
Collaborator Author

jeremypw commented Aug 5, 2023

Actually I am not sure that that issue is part of this PR since this is focused at resorting after adding/renaming a file. When a file is removed, the sort order does not change. I could probably fix the issue by resorting after deletion as well but not sure that is the most efficient way. I see whether there is a simpler way of getting the view to redraw after deletion first (it does after a terminal delete so ...)

@jeremypw
Copy link
Collaborator Author

jeremypw commented Aug 5, 2023

OK, so the problem is that the backup file is a hidden file and should not be visible anyway if "show-hidden" is off (and should not be added to the model). If "show-hidden" is on, then it gets removed as expected.

@jeremypw
Copy link
Collaborator Author

jeremypw commented Aug 5, 2023

Looks like this regression was introduced by https://github.com/elementary/files/pull/2200/files 😞
Need to check for ~ as a suffix as well.

@jeremypw
Copy link
Collaborator Author

jeremypw commented Aug 5, 2023

@zeebok I have added the fix for the issue you noticed to this PR to save time, as it is relatively small, but should strictly be a separate PR. Please let me know if you want it separated.

Copy link
Contributor

@zeebok zeebok left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well that seems to fix the issue I was seeing of the readme temp file being properly sorted with everything else. To me that fits this PR. I can open a separate issue for not seeing the .goutputstream files disappear even though they are deleted until some kind of view refresh.

@jeremypw
Copy link
Collaborator Author

jeremypw commented Aug 6, 2023

The .goutputstream issue was supposed to have been fixed by #2200 so that is another regression. However, lets merge this and I'll fix that separately.

@jeremypw jeremypw merged commit 82d5ae4 into main Aug 6, 2023
@jeremypw jeremypw deleted the fix-refresh-after-op branch August 6, 2023 08:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants