Unsaved changes indicator in editor tab

[austincondiff]

Description

When users make any change to a file without saving and autosave is turned off, an indicator will appear in the tab until the file is saved to let the user know there are unsaved changes.

Related Issues

• ✨ Modified file indicator #1437

Checklist

• I read and understood the contributing guide as well as the code of conduct
• The issues this PR addresses are related to each other
• My changes generate no new warnings
• My code builds and runs on my machine
• My changes are all related to the related issue above
• I documented my code

Screenshots

[tom-ludwig]

I'd be glad to investigate this issue and try to identify the bug. Could you please assign the issue to me?

[austincondiff]

@activcoding I've added you to the PR. I've put the unsaved-changes-tab-indicator branch in this repo so you can branch off of it. Create a PR that merges into this PR and then we can merge this one into main.

[avinizhanov]

@austincondiff

I checked where is the problem, there are actually 2 problems:

First problem with:

.onReceive(item.fileDocument?.$isDirty.eraseToAnyPublisher() ?? Empty().eraseToAnyPublisher()) { _ in
    isDirty = item.fileDocument?.isDirty ?? false
}

item.fileDocument?.isDirty is using old value, it's easy to fix with:

.eraseToAnyPublisher()) { newValue in
    isDirty = newValue
}

Second problem is that at point when onReceive created fileDocument is nil, so it's not listening until tab is re-rendered (for example on hover).

This one a bit harder to fix (at least for me), since fileDocument is just a property and it's not published.

I created branch on my account with fixes to both problems - unsaved-changes-tab-indicator...avinizhanov:CodeEdit:unsaved-changes-tab-indicator

var fileDocument: CodeFileDocument? {
        didSet {
            fileDocumentSubject.send()
        }
    }
var fileDocumentPublisher: AnyPublisher<Void, Never> {
        fileDocumentSubject.eraseToAnyPublisher()
    }
let fileDocumentSubject = PassthroughSubject<Void, Never>()

So we can listen to fileDocument changes and force re-render.
I also moved @State var isDirty and listeners to separate view EditorFileTabCloseButton, so it re-renders only this view and not whole tab.

Works good on my machine.

If you ok with these changes I can create a PR.

[austincondiff]

@avinizhanov that is fantastic, yeah, create the PR! I also discovered isDocumentEdited on NSDocument that does essentially what we are doing with isDirty. I am not sure if it is better to use this or our own variable. I am not sure if this is set back to false once we undo our first edit though. Do you have any thoughts around this?

[avinizhanov]

@austincondiff I didn't know there is isDocumentEdited on NSDocument .
I did a quick test and looks like it has same value as isDirty. I think we should use isDocumentEdited, but I'm not sure how to listen to changes to this variable.

[tom-ludwig]

Thanks for your solution, @avinizhanov. I only noticed a minor bug – when I press 'Enter' to add a new line, it doesn't detect a file change. Is this the intended behavior? I ask because in VSCode, a new line is considered an edit.

[austincondiff]

@avinizhanov that is where @activcoding and I were at. We weren't sure how to listen for changes either. Let us know if you find anything out. If we have to though, we can merge your PR in for the time being until we find a better solution.

[avinizhanov]

@austincondiff I'll try to find a way to listen for isDocumentEdited changes.

[avinizhanov]

@activcoding

I only noticed a minor bug – when I press 'Enter' to add a new line, it doesn't detect a file change. Is this the intended behavior? I ask because in VSCode, a new line is considered an edit.

It's not intended, I missed that.

This not triggered on enter:

codeFile
            .$content
            .dropFirst()
            .sink { _ in
                print("send")
                // Publish only when changed
                if !codeFile.isDirty {
                    codeFile.isDirty = true
                }
            }
            .store(in: &cancellables)

[avinizhanov]

@austincondiff #1449

The only solution I found is by overriding:

func updateChangeCount(_ change: NSDocument.ChangeType)
func updateChangeCount(withToken changeCountToken: Any, for saveOperation: NSDocument.SaveOperationType)

and creating custom publisher. First one is called when document is changed, second one when document is saved.

Note: To test this you need to disable autosave in settings, currently autosave triggered on each update, which updates isDocumentEdited to false. I think autosave should be triggered when you switch to other tab or minimize window.

@activcoding

Enter key bug is not fixed. Looks like it's a bug (or feature) of STTextView. STTextViewDelegate.textViewDidChangeText not called on enter key.

[austincondiff]

Yeah we figured it might need to be overridden. Thanks for carrying us to the finish line! 🏁

[austincondiff]

@avinizhanov isDocumentEdited is not set back to false when making a change and then undoing it. It is important that we hide the unsaved indicator when returning to the last saved state in your undo history. Any ideas how we cans do this?

Also it should also show when undoing past the saved state. So confer the following scenario.

• Open - indicator hidden
• Save - indicator hidden
• Change - indicator visible
• Undo - indicator hidden
• Undo - indicator visible

It currently is like this...

• Open - indicator hidden
• Change - indicator visible
• Save - indicator hidden
• Change - indicator visible
• Undo - indicator visible
• Undo - indicator visible

[avinizhanov]

@austincondiff

This will require somehow use same CEUndoManager.DelegatedUndoManager in CodeFileDocument.undoManager and in STTextViewController

Also CEUndoManager::undo() must publish events for each change:

NotificationCenter.default.post(name: .NSUndoManagerWillUndoChange, object: self.manager)
textView.applyMutationNoUndo(mutation.inverse)
NotificationCenter.default.post(name: .NSUndoManagerDidUndoChange, object: self.manager)

I did a test and it works as in description:

Screen.Recording.2023-10-14.at.7.42.48.PM.mov

Adding events to CEUndoManager::undo() CEUndoManager::redo() is easy.
But I'm not sure how to correctly get CEUndoManager.DelegatedUndoManager in CodeFileView

May be CEUndoManager should be created in CodeFileView and then pass it to CodeEditTextView, but it also needs SFTextView

[austincondiff]

Thanks for looking into this. @thecoolwinter is working on a STTextView replacement, so this may be something that we take into account.

[avinizhanov]

@austincondiff I think this is not related to STTextView, if replacement will still use CEUndoManager.

What we can do is make CEUndoManager public.

change textView: STTextView to optional textView:STTextView? (or new replaced TextView)

Create CEUndoManager in CodeFileView and pass it to CodeFileDocument and CodeEditTextView/STTextViewController

STTextViewController will set text view on loadView (where currently CEUndoManager is created)

[avinizhanov]

@austincondiff

#1451
https://github.com/CodeEditApp/CodeEditTextView/pull/212

[avinizhanov]

@austincondiff Lint failed on same file as in #1452

FindNavigatorModeSelector.swift:42:21: error: Control Statement Violation: `if`, `for`, `guard`, `switch`, `while`, and `catch` statements shouldn't unnecessarily wrap their conditionals or arguments in parentheses (control_statement)

[austincondiff]

Not anymore! 😆

[austincondiff]

In my testing, it works as expected. Some things I've discovered that do not work that will need to be addressed later on:

• Saving is not possible if using new windowing - 🐞 Cannot save file when opened as folder #1447
• If attempting to close an unsaved file, you get an alert to confirm the action but your selection does not work and often times crashes the app.
• When saving a file, sometimes it saves, others it does not (could be subsequent times to the first) but the indicator is not present after saving so I assume it was.

[avinizhanov]

@austincondiff

If attempting to close an unsaved file, you get an alert to confirm the action but your selection does not work and often times crashes the app.

I think I fixed this one. I can create separate PR when this one merged.

Screen.Recording.2023-10-16.at.5.24.22.PM.mov

[austincondiff]

@avinizhanov Very nice! Go for it, we just merged