vkbo / novelWriter

novelWriter is an open source plain text editor designed for writing novels. It supports a minimal markdown-like syntax for formatting text. It is written with Python 3 (3.9+) and Qt 5 (5.15) for cross-platform support.
https://novelwriter.io
GNU General Public License v3.0
2.01k stars 102 forks source link

Text import lost after Replace Straight Single Quotes/Switch editor #1979

Closed awqk closed 2 months ago

awqk commented 2 months ago

Steps to reproduce:

  1. create new document - novel document
  2. import text from file (anything)
  3. select all
  4. choose replace straight single quotes from format menu
  5. switch editor to any other document
  6. switch back to new document --> the new document is empty
awqk commented 2 months ago

import and replace are not part of the undo history, so the switch to another document probably doesn't realize there have been changes.

vkbo commented 2 months ago

I cannot reproduce from your steps, but however it is you're doing this, you're probably right that it thinks there are no changes.

vkbo commented 2 months ago

When I'm testing importing text, the LED in the status bar indicating document change is flipping to red, so it definitely registers the change of content, so that doesn't hold up. I'm also not sure what the replace quotes should have to do with it. Is that the only function that affects it, or is that just an example?

vkbo commented 2 months ago

Here is what inserting text actually does:

    def replaceText(self, text: str) -> None:
        """Replace the text of the current document with the provided
        text. This also clears undo history.
        """
        QApplication.setOverrideCursor(QCursor(Qt.CursorShape.WaitCursor))
        self.setPlainText(text)
        self.updateDocMargins()
        self.setDocumentChanged(True)
        QApplication.restoreOverrideCursor()
        return

So, it clears undo history, which is what setPlainText does (it's a Qt standard function), but it also forces the document changed flag, so whenever you switch document, it should be automatically saved.

Does the replace quotes feature actually make any changes? Or is the imported text clean of quote symbols?

awqk commented 2 months ago

I think I know what happened. 'Create new' does not switch the editor to the new document and the import is applied to some other document, likewise the replace afterwards.

This can go unnoticed, if the document editor is switched to an empty document, or even to an empty document in the trash.

In other words, a pilot error, sorry about this.

You probably did actively switch to the new document between steps 1 and 2.

awqk commented 2 months ago

possible enhancements:

vkbo commented 2 months ago
  • don't allow editing trash

It used to be blocked, but it was removed. You should be able to edit any document.

  • 'create new' should switch to the new document, I suppose this is expected behaviour.

Absolutely not. Opening a document causes whatever was in the editor to be saved, closed, and have its undo history cleared. That would be a very annoying feature in most cases. It's better to have then open document action only trigger by user choice.

  • don't allow the user to pick a document for import from text, when no document is open in the editor.

This is already the case, and pops up a dialog that states: "Please open a document to import the text file into."

vkbo commented 2 months ago

You probably did actively switch to the new document between steps 1 and 2.

Of course, otherwise I cannot import into it.

In other words, a pilot error, sorry about this.

Sure, so I guess this can be closed then.

I have been thinking about redesigning the import feature. Right now it imports the text into the open document, but instead it should insert the text at the cursor position. The current feature is technically a replace text feature, which is the safest way to ensure it doesn't bring in any formatting, but it's limited

The plan was to extend the import feature in parallel with new export features.

awqk commented 2 months ago

I just discarded a lengthy reply. Here's the summary: Do this redesign.

Supporting import to an existing non-empty document does not make sense when everything gets overwritten anyway.

Edit: oops, you have been too fast for me.

vkbo commented 2 months ago

Supporting import to an existing non-empty document does not make sense when everything gets overwritten anyway.

It doesn't make a lot of sense, sure. It warns you if the document has text, but I saw no reason to actively block it if that's what you're trying to do.

Edit: oops, you have been too fast for me.

Well, I responded an hour and a half ago. 😄