Improve IEx autocompletion to handle variable names.

[myronmarston]

Follow up to #5488.

[josevalim]

Please indent this as follows:

defp to_entries(%{kind: kind, name: name}) when
     kind in [:map_key, :module, :var_name] do

Same below. :)

[myronmarston]

Done.

[eksperimental]

@josevalim is this the new preferred way of breaking up guards? leaving when at the end of the first line?

[josevalim]

Please make sure that variable names in bindings are atoms, they are not always atoms in bindings because of hygiene, and then use Atom.to_string/1.

[myronmarston]

Good to know -- I had no idea! Is it worth adding test coverage for this since it's a simple mistake to make? What's an example of an expression that would result in a variable name not being an atom?

[josevalim]

Any expression that defines a variable inside a quote:

iex(1)> Code.eval_quoted (quote do var = 1 end), []
{1, [{{:var, Elixir}, 1}]}

One suggestion is to define a macro inside the test module and invoke that macro on @tag previous_line: "Mod.macro".

[myronmarston]

@josevalim I can't get your suggestion to work. I tried this:

defmacro define_var do
  quote do: my_var_1 = 1
end

@tag previous_line: "require #{__MODULE__}; #{__MODULE__}.define_var(); my_var_2 = 2"
test "ignores quoted variables when performing variable completion" do
  assert expand('my_var') == {:yes, '_2', []}
end

The test passes (which is good) but still passes if I undo my "filter to only atom variables" fix. I added some debug printing and found that the binding is just [my_var_2: 2] -- meaning that the variable inside the macro isn't affecting the evaulator's binding at all.

Thoughts?

[josevalim]

@myronmarston I will investigate.

[myronmarston]

@myronmarston I will investigate.

Great, thanks.

BTW, I've got the custom import auto-completion ready to go now, too. Do you want me to push that commit to this PR so it is included or should I wait until this is merged and then open a new PR? It builds on top of this one so I can't open another PR without also including the commits from this one.

[josevalim]

@myronmarston using var!(my_var_1, Elixir) when reading and writing should be enough to make the code fail. We give an integer context for other variables (truly temporary) and those are discarded from the binding.

[josevalim]

Let's wait until this is merged since I think there should be nothing more blocking it.

[myronmarston]

Thanks, the var! bit worked. However, I was surprised to discover that my guard clause in the for comprehension did not work to filter out non-atom variable names:

for {var_name, _value} when is_atom(var_name) <- binding,
# ...

Instead, I had to use a filter clause:

for {var_name, _value} <- binding,
    is_atom(var_name),
# ...

Is that expected? It certainly surprised me.

[josevalim]

That's a bug. Or we should fail to compile or it should work. Please open up an issue.

[josevalim]

@myronmarston fwiw, i cannot reproduce it:

iex(1)> for {k, v} when is_atom(v) <- [foo: :bar, baz: 1], do: v
[:bar]

[myronmarston]

Thanks for fixing the for comprehension bug, @josevalim! Do you want me to rebase against that and revert back to using a guard clause to filter to just atoms?

[josevalim]

❤️ 💚 💙 💛 💜