CQ-editor icon indicating copy to clipboard operation
CQ-editor copied to clipboard

fix #338 Current document is not saved

Open aavogt opened this issue 1 year ago • 1 comments
trafficstars

This makes _file_changed() consistent with load_from_file(). With this change, C-s C-o goes straight to the file picker after doing something in the editor widget.

I noticed that save ends up calling triggerRerender.emit(True) both directly and through _file_changed(), but maybe that doesn't matter because I don't notice any slowness or flickering in the UI.

aavogt avatar Apr 16 '24 15:04 aavogt

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 88.69%. Comparing base (03df6fa) to head (2f8c551).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #433   +/-   ##
=======================================
  Coverage   88.68%   88.69%           
=======================================
  Files          19       19           
  Lines        1564     1565    +1     
  Branches      190      190           
=======================================
+ Hits         1387     1388    +1     
  Misses        143      143           
  Partials       34       34           

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Apr 16 '24 16:04 codecov[bot]

I was about to raise a PR for the exact same thing! Good thing I checked PRs first.

The patch I was going to raise was similar to this one, except for consistency I replaced the raw set_text_from_file() call with the load_from_file wrapper:

diff --git a/cq_editor/widgets/editor.py b/cq_editor/widgets/editor.py
index eb70537..d2f1701 100644
--- a/cq_editor/widgets/editor.py
+++ b/cq_editor/widgets/editor.py
@@ -236,7 +236,8 @@ class Editor(CodeEditor,ComponentMixin):
     def _file_changed(self):
         # neovim writes a file by removing it first so must re-add each time
         self._watch_paths()
-        self.set_text_from_file(self._filename)
+        # Do not call set_text_from_file() directly as it sets isModified()
+        self.load_from_file(self._filename)
         self.triggerRerender.emit(True)
 
     # Turn autoreload on/off.

I might go as far as saying set_text_from_file() should never be called directly and should be derived to an Exception or to just call load_from_file() to avoid similar mistakes in future, but that's not the immediate issue.

I was actually doing this to fix a different bug: the editor asks for save confirmation on quit if it has done an auto-reload. These were my steps to reproduce:

  1. Open CQ Editor
  2. Close the editor tab (view->editor uncheck)
  3. Create an empty file: touch empty.py and open it in CQ Editor
  4. Enable "automatic load and preview" using button on top bar
  5. Update the file externally: touch empty.py
  6. Close CQ Editor

A dialog closes up saying "Close without saving?" and I have to click "yes" to exit.

I tested that my patch above fixes the issue, and also tested OP's patch and confirmed it does the same thing.

Wren6991 avatar Jun 12 '24 22:06 Wren6991

Thanks @aavogt and @Wren6991, LGTM.

adam-urbanczyk avatar Jun 13 '24 19:06 adam-urbanczyk