Import missing classes by dantleech · Pull Request #843 · phpactor/phpactor

dantleech · 2019-11-24T13:54:26Z

Import all unresolvable class names in the current namespace.

Note that this does not work with the FZF input stategy.

We return a collection of actions to import the missing classes. The inputlist blocks and works as expected, the fzf doesn't seem to block,and it seems the choice diaglogues are all opened at the same time.

@elythyr I couldn't see an obvious way to make fzf blocking (if that is indeed the issue):

TODO:

"temporarily" hacked the list strategy to use a simple list when processing a collection.
It's prompoting for imported classes (which would have been ambiguous?)
Not related to this PR, but would be nice to fix the spacing issue when there are no existing class imports (as in screen recording below)

doc/refactorings.md

Refactoring to import all unresolvable classes

camilledejoye · 2019-11-24T18:19:05Z

@elythyr I couldn't see an obvious way to make fzf blocking (if that is indeed the issue):

Not that I'm aware of, fzf is meant to work asynchronously so that for long process the user can start filter the results as soon as they arrived.

So if I understand correctly the handler will resolve all the class name which are not imported et return them as a list of action.
Then you want to filter this list and for each item kept execute every action to import the class.

Then it should be possible but we might have to rework the way the fzf version of the inputlist was made so that we can pass it our own "sink".
The idea is to do something like what I have done with the references:

You create a dictionary with the value of each item to print as key and the action (as a funcref) as value
You pass the keys to this dict to fzf so the user can filter
You pass the dict to the "sink" so that you can retrieve the action for each selected item
In the sink you call the action

Don't hesitate if I'm not clear or you need help.

dantleech · 2019-11-26T22:03:11Z

With normal inputlist (excuse the bad screen recording);

With fzf method:

Note that above a collection of actions is passed, normally inputlist blocks, with FZF this does not work:

It's opening the fzf(s) in new term windows.
It doesn't block and the next action is dispatched immediately.

I can't see any option to make FZF non-blocking.

this should be possible with a sink

Not sure I understand how this helps -- the callback will be executed when the item is selected, but it's already too late because FZF didn't block and all the actions have already been executed.

dantleech · 2019-11-26T22:49:34Z

@elythyr I've added a hack to always use the inputlist strategy when iterating over a collection as I don't know how to fix the FZF input. Would of course be good to fix this.

camilledejoye · 2019-11-27T07:49:06Z

I will try to find some time.

It doesn't block and the next action is dispatched immediately.

That's what I don't understand, I though that we would receive the list of all unimported class as a collection of action allowing to import each one of them.

camilledejoye · 2019-11-27T07:50:56Z

plugin/phpactor.vim

    if a:actionName == "collection"
        for action in a:parameters["actions"]
+            let g:_phpactorRpcActionIsCollection = v:true
            let result = phpactor#_rpc_dispatch(action["name"], action["parameters"])


So you call phpactor#_rpc_dispatch for each action received, before the user have filter them ?

Ok I finally understand, you want the classic fzf window to open ant let the user choose which FQCN to import, like if the user did it manually.

I don't see a way to make that work, since fzf is aynchronous.
Maybe with a chained list of action, so that:

you dispatch the first one

the user choose the right class to import

the sink is called

it dispatch the import action with the choosen class (added the use statement)

it dispatch the next action in the list (which will in turn open fzf and ask for which fqcn to use)

Does it make sense ?

yes -- but this is where I'm confused - it think it does that already (it calls the callback handler when the items are selected?). But I confess to not fully understanding everything that's going on 👀

For me what appen is:

We receive the collection of action

In the loop we dispatch each action

The rpc handler call fzf for each of them

The first fzf window is open

Once opened the second one is open (or not because already one windows exists but it doesn't matter)

and again until the end of the loop

So we must not use fzf in a loop, we need to treat it like the chain I mentioned.
So that the user choose the class for the first one, and only once he made a choice we call the next action

I got a question, let's say that we are in a file with 3 classes not yet imported: A, B and C.
The classes A and C have multiple possible FQCN and need the user to choose, B on the other hand as only one possibility.

In the collection that the new handler returns, does all three actions are the same ?
Or do we have two actions to choose the right FQCN and one to import the only possibility for B ?

dantleech · 2019-11-28T16:41:12Z

It will return three actions. 2 will be asking for a selection and the third will be an action to import the class. The other two actions will delegate to the standard import class handler AFAIRC (away from keyboard)

…

On 27 November 2019 19:04:20 CET, Camille Dejoye ***@***.***> wrote: elythyr commented on this pull request. > @@ -536,7 +546,9 @@ function! phpactor#_rpc_dispatch(actionName, parameters) " >> collection if a:actionName == "collection" for action in a:parameters["actions"] + let g:_phpactorRpcActionIsCollection = v:true let result = phpactor#_rpc_dispatch(action["name"], action["parameters"]) I got a question, let's say that we are in a file with 3 classes not yet imported: A, B and C. The classes A and C have multiple possible FQCN and need the user to choose, B on the other hand as only one possibility. In the collection that the new handler returns, does all three actions are the same ? Or do we have two actions to choose the right FQCN and one to import the only possibility for B ? -- You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub: #843 (comment)

-- Sent from my Android device with K-9 Mail. Please excuse my brevity.

dantleech · 2019-12-01T10:54:03Z

Created an issue to resolve the FZF issue here: #845

This reverts commit 8988f78.

dantleech force-pushed the unresolvable_classes_import branch from e25f6a7 to 3555b00 Compare November 24, 2019 14:07

greg0ire reviewed Nov 24, 2019

View reviewed changes

doc/refactorings.md Outdated Show resolved Hide resolved

dantleech mentioned this pull request Nov 24, 2019

Import classes on current line #815

Closed

dantleech added 5 commits November 24, 2019 14:35

Import Unresolvable Classes

1c87f27

Refactoring to import all unresolvable classes

Updates composer lock

d9a16ff

Adds doc and rename to => missing classes

adb5e8d

Add closing underscore

46297d9

Updates lock

ff72b1e

dantleech force-pushed the unresolvable_classes_import branch from 2524aea to ff72b1e Compare November 24, 2019 14:35

dantleech changed the title ~~Unresolvable classes import~~ Import missing classes Nov 24, 2019

Revert erronous changes to configs

a3c3913

Taluu approved these changes Nov 25, 2019

View reviewed changes

Always use inputlist strategy for collections

63f41b5

Moves decision to use inputlist strategy to dedicated function

6dba766

camilledejoye reviewed Nov 27, 2019

View reviewed changes

dantleech merged commit 8988f78 into develop Dec 1, 2019

dantleech deleted the unresolvable_classes_import branch December 1, 2019 10:50

dantleech mentioned this pull request Dec 1, 2019

[develop] inputlist strategy is forced for collection actions when FZF is used #845

Closed

dantleech added a commit that referenced this pull request Dec 1, 2019

Revert "Import missing classes (#843)"

64e2f27

This reverts commit 8988f78.

dantleech mentioned this pull request Dec 1, 2019

Revert "Import missing classes" #846

Closed

dantleech restored the unresolvable_classes_import branch December 1, 2019 10:57

dantleech deleted the unresolvable_classes_import branch December 1, 2019 11:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Import missing classes#843

Import missing classes#843
dantleech merged 8 commits intodevelopfrom
unresolvable_classes_import

dantleech commented Nov 24, 2019 •

edited

Loading

Uh oh!

Uh oh!

camilledejoye commented Nov 24, 2019

Uh oh!

dantleech commented Nov 26, 2019 •

edited

Loading

Uh oh!

dantleech commented Nov 26, 2019 •

edited

Loading

Uh oh!

camilledejoye commented Nov 27, 2019

Uh oh!

camilledejoye Nov 27, 2019 •

edited

Loading

Uh oh!

dantleech Nov 27, 2019 •

edited

Loading

Uh oh!

camilledejoye Nov 27, 2019

Uh oh!

camilledejoye Nov 27, 2019

Uh oh!

dantleech commented Nov 28, 2019 via email

Uh oh!

dantleech commented Dec 1, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

dantleech commented Nov 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

camilledejoye commented Nov 24, 2019

Uh oh!

dantleech commented Nov 26, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dantleech commented Nov 26, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

camilledejoye commented Nov 27, 2019

Uh oh!

camilledejoye Nov 27, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dantleech Nov 27, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

camilledejoye Nov 27, 2019

Choose a reason for hiding this comment

Uh oh!

camilledejoye Nov 27, 2019

Choose a reason for hiding this comment

Uh oh!

dantleech commented Nov 28, 2019 via email

Uh oh!

dantleech commented Dec 1, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

dantleech commented Nov 24, 2019 •

edited

Loading

dantleech commented Nov 26, 2019 •

edited

Loading

dantleech commented Nov 26, 2019 •

edited

Loading

camilledejoye Nov 27, 2019 •

edited

Loading

dantleech Nov 27, 2019 •

edited

Loading