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

Dialog for saving conflicting changes from context file editor #402

Merged
merged 4 commits into from
Feb 26, 2025

Conversation

takluyver
Copy link
Member

Since #304, the context file editor periodically checks the file for changes and reloads the context file as long as you don't have any unsaved changes locally. This PR checks the file on disk when you try to save, and if it has changed, it tells you this and gives you options to deal with it:

image

Clicking 'View changes' opens a diff viewer (meld) to compare the file on disk (left) with your editor (right). You can pull changes across either way and save the files from there. When you close this view, any changes you saved on the right go back into the editor view.

image

I also wrapped the editor setText() method to restore the approximate scroll position when we reload the editor contents. This isn't super smart, but for small edits it works quite nicely, and I'm hoping it's strictly better than jumping up to the top.

@takluyver takluyver added the GUI For GUI-related things label Feb 20, 2025
Copy link

codecov bot commented Feb 20, 2025

Codecov Report

Attention: Patch coverage is 36.04651% with 55 lines in your changes missing coverage. Please review.

Project coverage is 75.65%. Comparing base (b06df67) to head (8039662).
Report is 36 commits behind head on master.

Files with missing lines Patch % Lines
damnit/gui/editor.py 28.57% 45 Missing ⚠️
damnit/gui/main_window.py 56.52% 10 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #402      +/-   ##
==========================================
+ Coverage   75.59%   75.65%   +0.06%     
==========================================
  Files          35       35              
  Lines        5912     6137     +225     
==========================================
+ Hits         4469     4643     +174     
- Misses       1443     1494      +51     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@matheuscteo matheuscteo self-requested a review February 26, 2025 12:50
@matheuscteo
Copy link
Member

Thanks Thomas, it looks good and worked fine from my side

my only observations are:

  • Are we sure that meld won't crash and be installed?
  • If the GUI crashes or someone closes it from the command line the temp file for the context.py won't be deleted.
  • Should we perhaps hide the temp file? I know it exists only for a short time, but wouldn't be nice if a third person tries to edit this one. I'd go for something like it happens for vim does (.test.swp)

Apart from that, LGTM

@takluyver
Copy link
Member Author

Are we sure that meld won't crash and be installed?

It's definitely installed on Maxwell, and I think it's probably the most likely graphical diff viewer to be installed on a random Linux system. We could make it configurable, or have a list of different ones to try, but I'm inclined to keep things simple until it becomes an issue.

Meld has been pretty reliable from what I remember of it, but it's possible it could crash. Any error output would be visible in the terminal where you launched DAMNIT.

Should we perhaps hide the temp file?

Yup, good idea. 🙂

If the GUI crashes or someone closes it from the command line the temp file for the context.py won't be deleted.

That's true. I don't think there's really a good way to avoid that, but if we let Python create the tempfile in the default /tmp folder, then any leftover files will at least be cleaned up when the machine reboots (and potentially sooner, if IT run some temp file cleaner daemon). I don't think these ones actually need to be in the amore folder.

@takluyver takluyver merged commit 7d11884 into master Feb 26, 2025
5 of 6 checks passed
@takluyver takluyver deleted the feat/save-conflict-dialog branch February 26, 2025 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GUI For GUI-related things
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants