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

:wq silently exits without an error if target file is not writable #1575

Closed
Pistahh opened this issue Jan 24, 2022 · 3 comments · Fixed by #2267
Closed

:wq silently exits without an error if target file is not writable #1575

Pistahh opened this issue Jan 24, 2022 · 3 comments · Fixed by #2267
Labels
A-helix-term Area: Helix term improvements C-bug Category: This is a bug E-good-first-issue Call for participation: Issues suitable for new contributors

Comments

@Pistahh
Copy link

Pistahh commented Jan 24, 2022

Reproduction steps

:wq should fail and indicate if it couldn't save the file but it doesn't, it just exits.

  1. edit a file that you have no permissions to, e.g. /etc/hosts as non-root
  2. Try to save it and exit with :wq
  3. watch as hx exits without any indication that it couldn't save the changes
  4. inspect /etc/hosts that it indeed wasn't changed

Environment

  • Platform: Linux
  • Terminal emulator: gnome terminal
  • Helix version: v0.5.0-186-g371c84f
@Pistahh Pistahh added the C-bug Category: This is a bug label Jan 24, 2022
@Pistahh Pistahh changed the title :wq silently exists without an error if target file is not writable :wq silently exits without an error if target file is not writable Jan 24, 2022
@kirawi kirawi added A-helix-term Area: Helix term improvements E-good-first-issue Call for participation: Issues suitable for new contributors labels Jan 24, 2022
@dead10ck
Copy link
Member

I started trying to look into this one. At first glance, it seems like this should exit with an error before quitting. However, if we look at write_impl, we can see that it looks like it just adds an async job and returns immediately, which would explain why it just proceeds to quit, and doesn't really care if the write succeeds or not:

let future = doc.format_and_save(fmt);
cx.jobs.add(Job::new(future).wait_before_exiting());
if path.is_some() {
let id = doc.id();
let _ = cx.editor.refresh_language_server(id);
}
Ok(())

I thought it would make sense to wait on the job in between calling write and calling quit, so looking at the Jobs type, it looks like there is a function for waiting on all the pending jobs:

/// Blocks until all the jobs that need to be waited on are done.
pub fn finish(&mut self) {
let wait_futures = std::mem::take(&mut self.wait_futures);
helix_lsp::block_on(wait_futures.for_each(|_| future::ready(())));
}

However, as we can see, this doesn't care about the result of the jobs either. Would it make sense to have this function return a Result that's an error if any of the pending jobs error, and Ok otherwise?

@DannyJJK
Copy link

DannyJJK commented Mar 30, 2022

I'd love to try using Helix as my main editor, but this single issue makes me nervous to use it. There's various reasons a file could fail to write that are not just permissions based, I wouldn't want to think I'd saved a file successfully to find that no changes have been made. Has there been any further progress on this?

@dead10ck
Copy link
Member

I agree, this seems rather important from a reliability perspective. I'll take a crack at the approach I mentioned above.

dead10ck added a commit to dead10ck/helix that referenced this issue Apr 25, 2022
During write-quit, if the file fails to be written for any reason, helix
will still quit without saving the changes. This fixes this behavior by
introducing fallibility to the asynchronous job queues. This will also
benefit all contexts which may depend on these job queues.

Fixes helix-editor#1575
dead10ck added a commit to dead10ck/helix that referenced this issue Apr 25, 2022
During write-quit, if the file fails to be written for any reason, helix
will still quit without saving the changes. This fixes this behavior by
introducing fallibility to the asynchronous job queues. This will also
benefit all contexts which may depend on these job queues.

Fixes helix-editor#1575
dead10ck added a commit to dead10ck/helix that referenced this issue Apr 30, 2022
During write-quit, if the file fails to be written for any reason, helix
will still quit without saving the changes. This fixes this behavior by
introducing fallibility to the asynchronous job queues. This will also
benefit all contexts which may depend on these job queues.

Fixes helix-editor#1575
dead10ck added a commit to dead10ck/helix that referenced this issue May 1, 2022
During write-quit, if the file fails to be written for any reason, helix
will still quit without saving the changes. This fixes this behavior by
introducing fallibility to the asynchronous job queues. This will also
benefit all contexts which may depend on these job queues.

Fixes helix-editor#1575
dead10ck added a commit to dead10ck/helix that referenced this issue May 1, 2022
During write-quit, if the file fails to be written for any reason, helix
will still quit without saving the changes. This fixes this behavior by
introducing fallibility to the asynchronous job queues. This will also
benefit all contexts which may depend on these job queues.

Fixes helix-editor#1575
dead10ck added a commit to dead10ck/helix that referenced this issue May 13, 2022
During write-quit, if the file fails to be written for any reason, helix
will still quit without saving the changes. This fixes this behavior by
introducing fallibility to the asynchronous job queues. This will also
benefit all contexts which may depend on these job queues.

Fixes helix-editor#1575
dead10ck added a commit to dead10ck/helix that referenced this issue May 17, 2022
During write-quit, if the file fails to be written for any reason, helix
will still quit without saving the changes. This fixes this behavior by
introducing fallibility to the asynchronous job queues. This will also
benefit all contexts which may depend on these job queues.

Fixes helix-editor#1575
dead10ck added a commit to dead10ck/helix that referenced this issue May 22, 2022
During write-quit, if the file fails to be written for any reason, helix
will still quit without saving the changes. This fixes this behavior by
introducing fallibility to the asynchronous job queues. This will also
benefit all contexts which may depend on these job queues.

Fixes helix-editor#1575
dead10ck added a commit to dead10ck/helix that referenced this issue May 23, 2022
During write-quit, if the file fails to be written for any reason, helix
will still quit without saving the changes. This fixes this behavior by
introducing fallibility to the asynchronous job queues. This will also
benefit all contexts which may depend on these job queues.

Fixes helix-editor#1575
dead10ck added a commit to dead10ck/helix that referenced this issue May 23, 2022
During write-quit, if the file fails to be written for any reason, helix
will still quit without saving the changes. This fixes this behavior by
introducing fallibility to the asynchronous job queues. This will also
benefit all contexts which may depend on these job queues.

Fixes helix-editor#1575
dead10ck added a commit to dead10ck/helix that referenced this issue May 29, 2022
During write-quit, if the file fails to be written for any reason, helix
will still quit without saving the changes. This fixes this behavior by
introducing fallibility to the asynchronous job queues. This will also
benefit all contexts which may depend on these job queues.

Fixes helix-editor#1575
dead10ck added a commit to dead10ck/helix that referenced this issue Jun 2, 2022
During write-quit, if the file fails to be written for any reason, helix
will still quit without saving the changes. This fixes this behavior by
introducing fallibility to the asynchronous job queues. This will also
benefit all contexts which may depend on these job queues.

Fixes helix-editor#1575
dead10ck added a commit to dead10ck/helix that referenced this issue Jun 4, 2022
During write-quit, if the file fails to be written for any reason, helix
will still quit without saving the changes. This fixes this behavior by
introducing fallibility to the asynchronous job queues. This will also
benefit all contexts which may depend on these job queues.

Fixes helix-editor#1575
dead10ck added a commit to dead10ck/helix that referenced this issue Jun 4, 2022
During write-quit, if the file fails to be written for any reason, helix
will still quit without saving the changes. This fixes this behavior by
introducing fallibility to the asynchronous job queues. This will also
benefit all contexts which may depend on these job queues.

Fixes helix-editor#1575
dead10ck added a commit to dead10ck/helix that referenced this issue Jun 14, 2022
During write-quit, if the file fails to be written for any reason, helix
will still quit without saving the changes. This fixes this behavior by
introducing fallibility to the asynchronous job queues. This will also
benefit all contexts which may depend on these job queues.

Fixes helix-editor#1575
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-helix-term Area: Helix term improvements C-bug Category: This is a bug E-good-first-issue Call for participation: Issues suitable for new contributors
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants