Skip to content

Conversation

zachdaniel
Copy link
Contributor

@zachdaniel zachdaniel commented Sep 27, 2025

For example:

If you pasted this into an iex shell, you might have a very very bad day.

MyApp.Accounts.User
|> Ecto.Query.where(user], user.id == 1) # note the missing open bracket
|> MyApp.Repo.delete_all()

This is still rough, things I'm not sure about:

  1. I'm not sure how we feel about the method of just rewriting the evaluated code to check for this condition.
  2. I'm trying to display the operator, but it's using the tokenized representation of it and I couldn't find a mapper for that. I can just not display the operator, or I can write a mapping function myself.
  3. I should probably colorize and indent the message properly, but wanted to validate the approach before pushing further.

In practice it looks like this:

Interactive Elixir (1.20.0-dev) - press Ctrl+C to exit (type h() ENTER for help)
iex(1)> 1
1
iex(2)> |> Kernel.+-(1)
** (ArgumentError) cannot pipe v(-1) into Kernel.+() - 1, the :- operator can only take one argument
    (elixir 1.20.0-dev) lib/macro.ex:341: Macro.pipe/3
    (stdlib 7.0.2) lists.erl:2466: :lists.foldl/3
    (elixir 1.20.0-dev) expanding macro: Kernel.|>/2
    iex:2: (file)
iex(2)> |> Kernel.+(1)
Skippping evaluation of:

|> Kernel.+(1)

For safety, you cannot begin an expression with `arrow_op` when the last expression was an error.
iex(3)>
CleanShot.2025-09-27.at.10.31.14.mp4

For example:

If you pasted this into an iex shell, you might have a very very bad
day.

```elixir
MyApp.Accounts.User
|> Ecto.Query.where(user], user.id == 1) # note the missing open bracket
|> MyApp.Repo.delete_all()
```
@josevalim
Copy link
Member

Looks promising! I think you can simplify things a bit. I don't think we need, for example, to repeat the expression, it should be clear it is the one right above.

to handle syntax errors, send to the evaluator to set `iex_error` to
`:force`
if its set to `:force`, we downgrate it to `true` first.
@zachdaniel
Copy link
Contributor Author

Okay, I've done some cleanup. I realized we weren't handling syntax errors (i.e errors not coming from evaluating the code) and I had to do something a bit hacky to make that work.

I send code to the evaluator to Process.put(:iex_error, :force). It is using :force, because if it just set true, the evaluator would immediately delete it after successfully evaluating it. This way if it's set to :force, the evaluator sees that and sets it to true. I'm not a fan of this but it was the quickest path to "a" solution.

Ready for another look given the simplification and the parse error handling concept.

@josevalim
Copy link
Member

One last comment to clean up the interfaces. Do you think we could have some tests? Perhaps in interaction_test.exs?

@zachdaniel
Copy link
Contributor Author

Yep, will wrap up and add tests later today or tomorrow 👌

@zachdaniel
Copy link
Contributor Author

Tests added 👍

@josevalim
Copy link
Member

Beautifully done. Will be merged once CI passes!

@josevalim josevalim merged commit f310ed9 into elixir-lang:main Sep 28, 2025
13 checks passed
@josevalim
Copy link
Member

💚 💙 💜 💛 ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants