A file is considered modified even if contents are the same as before any modifications

[jakubkaczor]

Summary

I expect no indicator showing file change if the file content is the same as the buffer content. This is not always the case. Let us have Helix open on a save file, nothing is modified. Then, doing some change, and formatting with :format makes the file considered modified even if :format reduced it to the state it was in before any actions.

Reproduction Steps

1. Open a file.
2. Modify a file in a way that will be reverted by :format.
3. Execute :format.

Helix log

I don't think it will be useful, but here is ~/.cache/helix/helix.log

2024-07-23T23:44:02.890 helix_lsp::transport [ERROR] rust-analyzer err <- "[ERROR rust_analyzer::config] failed to find any projects in [AbsPathBuf(\"/tmp/tmp.O8NnHkaAmP\")]\n"
2024-07-23T23:44:02.896 helix_lsp::transport [ERROR] rust-analyzer err <- "[ERROR rust_analyzer::lsp_utils] rust-analyzer failed to discover workspace\n"
2024-07-23T23:44:02.896 helix_lsp::transport [ERROR] rust-analyzer err <- "[ERROR rust_analyzer::lsp_utils] rust-analyzer failed to discover workspace\n"
2024-07-23T23:44:02.896 helix_term::application [WARN] unhandled window/showMessage: ShowMessageParams { typ: Error, message: "rust-analyzer failed to discover workspace" }
2024-07-23T23:44:02.896 helix_term::application [WARN] unhandled window/showMessage: ShowMessageParams { typ: Error, message: "rust-analyzer failed to discover workspace" }```

</details>


### Platform

Linux

### Terminal Emulator

Alacritty

### Installation Method

nixpkgs

### Helix Version

helix 24.7 (079f5442)

[pascalkuthe]

Thus is intentional behavior, the modification indicator does not indicate whether a file is incidentally the same as another file but whether the document in undo history that was saved to/read from disk.

[jakubkaczor]

Alright. Then, I would like to request a change of this behavior. To me, it appears indifferent to write to the file if the contents are the same, even worse being warned about not doing so on quitting. Should I create a feature request, should you convert the discussion into it, or should it be left as it is? I suppose the current behavior is guided by simpler implementation, isn't it? Comparing contents to be robust would require accounting for side effects uninitiated by Helix. If I remember correctly, this is not yet established and was mentioned, for example, in the issue about updating VCS gutter.

[the-mikedavis]

Our behavior matches NeoVim, Kakoune and VSCode. We don't do what you propose for the same reason as the other editors: it's expensive. In order to reset the modification indicator when the buffer matches the underlying file you need to compare the buffer to on-disk contents per change which is very wasteful resource-wise regardless of file size, but especially for large files. #4994 is about using file system notifications to rerun the diff detection (via gix). Even if you used file-system notifications to detect when the underlying file changed you would need to keep a copy of the file in memory for comparisons.

[jakubkaczor]

I don't consider which editors match the behavior. I take a fresh look for Helix.

We don't do what you propose for the same reason as the other editors: it's expensive.

I am not sure what exactly you consider expensive. I will assume buffer-file comparison. I consider it expensive too, but only if it is done often. I believe it needs to be done only, if the file is modified in a way unknown to Helix. Otherwise, couldn't change be cheaply inferred from the operations sequence?

[the-mikedavis]

I would consider it too expensive if it's done often or in the main thread. The diff gutter is calculated asynchronously for example. It should be possible to detect when the current changes reduce the diff from the on-disk file to nothing, though it would take some extra memory to track that. We would need to move the calculation of the modification indicator off the main thread though. The current modification indicator has a meaning - the current undo checkpoint is what was last written to- or read from disk - and is minimally expensive and also very simple implementation-wise (see here). Feel free to hack on this if you're interested but I don't think it's something we'd be interested in merging.

[jakubkaczor]

I see. Thanks for pointing to the item and explanations.

Feel free to hack on this if you're interested but I don't think it's something we'd be interested in merging.

I understand this is too much for the main thread, but if dispatched to another, would you still not consider it for merging? Why? Would then still be some aspect you consider too expensive? Can you imagine a form of the feature implementation you would consider?

[the-mikedavis]

Once you work around how expensive it is to compare the implementation is undesirably complicated. And seeing the modification indicator update asynchronously would be confusing IMO. So no I don't see a version of this that I would be interested in merging.