abingham / emacs-ycmd

Emacs client for ycmd, the code completion system.
MIT License
384 stars 46 forks source link

FixIt diffs file instead of buffer #474

Closed zedeus closed 6 years ago

zedeus commented 6 years ago

When ycmd parses a C buffer with changes and suggests a FixIt, the chunk diffs shown use the unsaved file. This results in errors made after saving the file not being diffed properly, as well as showing unrelated changes. I haven't tested with languages other than C, so I don't know if this is a general issue.

Without saving: image

After saving: image

Debug info:

((python
  (executable . "/usr/bin/python")
  (version . "3.6.5"))
 (clang
  (has_support . t)
  (version . "clang version 6.0.0 (tags/RELEASE_600/final)"))
 (extra_conf
  (path . "/home/zed/.emacs.d/layers/+tools/ycmd/global_conf.py")
  (is_loaded . t))
 (completer
  (name . "C-family")
  (servers)
  (items
   ((key . "compilation database path")
    (value . "None"))
   ((key . "flags")
    (value . "['-std=c11', '-Wall', '-Wextra', '-pthread', '-I/usr/include/gtk-3.0', '-I/usr/include/pango-1.0', '-I/usr/include/glib-2.0', '-I/usr/lib/glib-2.0/include', '-I/usr/include/fribidi', '-I/usr/include/cairo', '-I/usr/include/pixman-1', '-I/usr/include/freetype2', '-I/usr/include/libpng16', '-I/usr/include/harfbuzz', '-I/usr/include/uuid', '-I/usr/include/gdk-pixbuf-2.0', '-I/usr/include/gio-unix-2.0/', '-I/usr/include/libdrm', '-I/usr/include/atk-1.0', '-I/usr/include/at-spi2-atk/2.0', '-I/usr/include/at-spi-2.0', '-I/usr/include/dbus-1.0', '-I/usr/lib/dbus-1.0/include', '-pthread', '-lgtk-3', '-lgdk-3', '-lpangocairo-1.0', '-lpango-1.0', '-lfribidi', '-latk-1.0', '-lcairo-gobject', '-lcairo', '-lgdk_pixbuf-2.0', '-lgio-2.0', '-lgobject-2.0', '-lglib-2.0', '-lmpdclient', '-resource-dir=/usr/share/ycmd/ycmd/../clang_includes', '-fspell-checking']"))
   ((key . "translation unit")
    (value . "[REDACTED]/main.c")))))

Server is running at: 127.0.0.1:36341

Ycmd Mode is enabled

--------------------

Ycmd version:   1.3snapshot (package: 20180520.353)
Emacs version:  27.0.50
System:         x86_64-pc-linux-gnu
Window system:  x
abingham commented 6 years ago

I've done a quick scan through our code and (what I think is) the appropriate code in YCMD itself, and I think emacs-ycmd is doing the right thing WRT to the ycmd protocol. We're sending the current buffer contents, it's expecting those, etc. I'm willing to assume for now that ycmd itself is doing the right thing regarding unsaved buffer contents. So...

@ptrv Is it possible that we're simply showing diffs against the saved file instead of the in-buffer contents? You know this part of the code much better than I do.

ptrv commented 6 years ago

@abingham Yes, you are right, we are showing the diff against the local copy of the file.

Here: https://github.com/abingham/emacs-ycmd/blob/1806b6a9d00946f06b42bb218b3839a392b8a95e/ycmd.el#L1596

We use the filepath to diff against current-buffer for diff-no-select and it should be the buffer from 5 lines above. This is due to diff-no-select is using the local copy if you provide the filepath instead of a buffer.

abingham commented 6 years ago

@zestyr I've pushed a change to the fix-fixit-diff branch. Could you give it a try and let me know if it works for you? I've taken it for a small test-drive, and it seems to work, but I'd like to verify that it solves your problem as well.

zedeus commented 6 years ago

@abingham It works indeed, thank you for the fix. Sorry for the delay.

abingham commented 6 years ago

No worries. Thanks for testing it out.