emacs-lsp / lsp-mode

Emacs client/library for the Language Server Protocol
https://emacs-lsp.github.io/lsp-mode
GNU General Public License v3.0
4.77k stars 883 forks source link

`willRenameFile` request on `save-buffer` sends backup file URI #3516

Open Walheimat opened 2 years ago

Walheimat commented 2 years ago

Thank you for the bug report

Bug description

When editing files using a language server that supports file renaming, saving a file sends a willRenameFiles request providing a new URI of the backup file. It seems that this is because over the course of save-buffer a rename happens by default.

I found this in the docs of make-backup-files:

Non-nil means make a backup of a file the first time it is saved. This can be done by renaming the file or by copying. Renaming means that Emacs renames the existing file so that it is a backup file, then writes the buffer into a new file. Any other names that the old file had will now refer to the backup file. The new file is owned by you and its group is defaulted.

Steps to reproduce

You will need a LSP server that supports willRename file operation. The only one I use is the Dart Analyzer via lsp-dart.

In the case of Dart, importing a file in another file and changing that file and saving it will update the import and make it point to the backup file.

Expected behavior

No such request is sent as no renaming took place outside of Emacs doing backups.

Which Language Server did you use?

lsp-dart (which is an external project I know but the only one I use that supports this).

OS

Linux

Error callstack

No response

Anything else?

No response

ericdallo commented 2 years ago

Hi @Walheimat, I can help with that, but I didn't get the issue, could you elaborate a little bit more on the step to step? When I implemented I tested it with both Dart and Clojure servers and it seemed to work

Walheimat commented 2 years ago

Yes, I'm currently trying to reproduce this on a different machine/project.

Walheimat commented 2 years ago

Okay, I can recreate this the following way (using Flutter):

  1. Create a project:
     flutter create example
  2. Open lib/main.dart and import root folder when prompted.
  3. Edit the file (empty line for example) and save.
  4. Test file <root>/test/widget_test.dart now imports:
    import '../../../../.emacs.d/backups/!home!karusster!tests!flutters!example!lib!main.dart~';

So it looks like this happens on first save, when the backup file is created. I assume this can be circumvented by setting backup-by-copying to t but will have to test first.


Can confirm that setting backup-by-copying to t seems to prevent this issue.


Note that by default backups are stored in the same directory, so if you don't set backup-directory-alist the import instead gets updated to:

import 'package:example/main.dart~';
ericdallo commented 2 years ago

@Walheimat that's weird, when did you rename a file so lsp-mode would send willRenameFiles request?

ericdallo commented 2 years ago

BTW I can't repro on doom-emacs, but backup-by-copying is t, probably setted by doom

Walheimat commented 2 years ago

The renaming happens when save-buffer is called but backup-by-copying is nil (which is vanilla Emacs default). Don't ask me why Emacs does this renaming shenanigans. I'm not sure I understand why it's necessary but that's what it does.

There's no renaming of the user, it's Emacs creating the backup and then renaming the original file to the name of the backup as the new backed up state, I assume.

ericdallo commented 2 years ago

Make sense! just not sure what lsp-mode should do, WDYT @yyoncho ?

Walheimat commented 2 years ago

I will just set backup-by-copying myself but other vanilla users might run into this issue. Maybe lsp-mode could warn if the server supports file renaming but the variable is nil?

yyoncho commented 2 years ago

I will just set backup-by-copying myself but other vanilla users might run into this issue. Maybe lsp-mode could warn if the server supports file renaming but the variable is nil?

Something like that and then suggest either disabling lsp-mode feature or changing the backup setting.

apergos commented 9 months ago

I just got bit by this bug, tracked it down to the backup files and willRenameFiles, vanilla emacs 29.1, lsp-mode 20231203.705 and I disabled backups of files with relevant extensions to work around it, since those files will end up in a git repo anyways. Only then did I find this open issue. In my case, the language server threw an exception because it didn't like the rename, which threw me off the track.

slipset commented 6 months ago

I experienced this bug as well when setting up with a new emacs config. Setting

(setq backup-by-copying t)

Seems to have resolved the issue.

justinbarclay commented 3 months ago

I get bit by this bug so often.