Auto import class after completion

[kermorgant]

Feature description

After completion with company-phpactor, if the selected candidate is a class (see "info" key in phpactor's output), a proper "use" statement is added after the namespace declaration in case it was absent.

In order to distinguish classes with the same name, fully qualified names are shown as annotations beside every completion candidate.

What is changed :

• completion candidates are given text properties with additionnal informations given by phpactor (info and type keys).
• phpactor is called after the completion if the candidate had type 't' (class) to add the "use" statement.
• use of functions from go-mode.el to apply a patch on current buffer instead of replacing the whole buffer content in order to save cursor position (Thanks to Dominik Honnef, go-mode author)
• also widen the buffer before completion, making completion also work during narrowing.

[kermorgant]

@zonuexe I saw the following comment in the code about restoring position. Could you please share the link to the gofmt implementation ?

[ejmr]

... [gofmt] code about restorting position.

Maybe this?

[zonuexe]

@ejmr
Yes. This code is copied to several packages such as format-sql and prettier-emacs.

[kermorgant]

According to phpactor's author, this is going to be revisited, but for now, I think it's the best we can do.

[kermorgant]

@ejmr @zonuexe Thanks for the references

I just "copied" a bunch of functions from the first link and only modified what seemed the most obvious to me, leaving all the magic details aside and got something working (proof)

I'll push it in that shape for the moment although it certainly requires many things to finalize that.

[kermorgant]

this fixes issue #35

[kermorgant]

buffer and file name to adapt

[kermorgant]

Hi @zonuexe

I can't see what to change about this PR (at least at the time of writing this). I've not used it much since I got it working only today, but it's been fine so far.

How about you, what do you think about maybe merging this, would something need to be adressed ?

[dominikh]

@kermorgant asked me, as go-mode's author, to sign off on the inclusion of these functions. If you wanted to abide by its license strictly, it'd require a copyright notice as well as the contents of the MIT license.

Seeing how these functions are trivial, however, and anyone implementing them from scratch would come up with identical code, I really don't care about the license and you're free to use them, with or without the reference to go-mode.

tl;dr: 👍

[ejmr]

I can't see what to change about this PR.

I have a few suggestions, none of which (except maybe the first) are so serious that I feel they should prevent merging.

1. I do not know how @zonuexe feels about this, but personally I do not feel any commit which explicitly says it is "WIP" (especially in the title line) belongs in a master branch. When coding I make such commits all the time, small little WIP commits as I work my way through whatever task I'm on. But once I am ready to push the branch out to the public I always use git-rebase to cleanup commit messages (e.g. removing "WIP" and elaborating on them, spell-checking, etc.), squash together small commits where it makes sense, and so on. If you work in a similar way then you may, as I do, find Git's "autosquash" feature quite useful.

2. Even if @dominikh considers it "trivial", I feel that any code taken from or inspired by his work should include his name in the comments next to the URL to the relevant project. There are many reasons why that URL could suddenly become invalid or "dead". There are far fewer scenarios under which his name would change though. So I feel like including him name is important to give proper credit and thanks. A line in the relevant commit message reading Special-Thanks: Dominik Honnef is also something I like to do personally in these situations.

3. I saw at least one defun which did not have a blank line above it; the function immediately followed some other defun. While yes, it's a trivial matter, I think adding that line of whitespace improves readability. Something I've learned over the years from this project and many others is that if you hold yourself to a high standard regarding readability, commit messages, etc., then other contributors will gradually adhere to the same standards, and that's good for the overall quality of any software project.

--ejmr

[kermorgant]

Thanks for the review.

I just addressed the first two comments but did not spot missing blank lines (could it be related to those "^L" characters ?).

[zonuexe]

@kermorgant
Sorry for the late reply, I will review this PR from now on.

Postscript: I saw an email this morning (Japan time), but I have not read the contents yet. sorry.

[zonuexe]

Please add candidate as lexical variable.

diff --git a/company-phpactor.el b/company-phpactor.el
index 44514fb..90bf850 100644
--- a/company-phpactor.el
+++ b/company-phpactor.el
@@ -52,7 +52,7 @@ Here we create a temporary syntax table in order to add $ to symbols."

 (defun company-phpactor--get-candidates ()
   "Build a list of candidates with text-properties extracted from phpactor's output."
-  (let ((suggestions (company-phpactor--get-suggestions)))
+  (let ((suggestions (company-phpactor--get-suggestions)) candidate)
     (mapcar
      (lambda (suggestion)
        (setq candidate (plist-get suggestion :name))

[zonuexe]

@kermorgant
This code seems to work well in my environment. 👍

I have invited you as a member of this organization. After removing WIP from the name of this PR, you can merge this branch at your own discretion.

[kermorgant]

@zonuexe
Thanks for the invitation. I changed the title and also squashed the commits as I consider the "lateral annotation" feature (pr #33) being a subset of this one.