Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cell formatting doesn't respect dependencieslocals_without_parens #1212

Closed
sabiwara opened this issue Jun 2, 2022 · 7 comments
Closed

Cell formatting doesn't respect dependencieslocals_without_parens #1212

sabiwara opened this issue Jun 2, 2022 · 7 comments

Comments

@sabiwara
Copy link
Contributor

sabiwara commented Jun 2, 2022

Environment

  • Elixir & Erlang/OTP versions (elixir --version): 1.13.4 / OTP 25
  • Operating system: MacOS
  • How have you started Livebook (mix phx.server, livebook CLI, Docker, etc): escript
  • Livebook version (use git rev-parse HEAD if running with mix): 0.6.1
  • Browsers that reproduce this bug (the more the merrier): Firefox
  • Include what is logged in the browser console: -
  • Include what is logged to the server console: -

Current behavior

Adding ecto and input the following query:

from f in "foo", select: f.name

Screen Shot 2022-06-02 at 10 08 56

Run the formatter:

Screen Shot 2022-06-02 at 10 09 08

It added parentheses despite locals_without_parens.

Expected behavior

Do not add parentheses for macros specified in locals_without_parens.

I'm happy to have a look and try to open a PR if this is something we want to address.

@josevalim
Copy link
Contributor

I think this is hard because we would need to list somewhere the dependencies we want to include on the formatting. @jonatanklosko who does the formatting? Livebook or the runtime? If Livebook, then it is not possible at all and we should close this as not possible. :(

@jonatanklosko
Copy link
Member

@josevalim it's the runtime, is there an easy way for us to get all dependency formatting rules, without specifying them explicitly?

@sabiwara
Copy link
Contributor Author

sabiwara commented Jun 2, 2022

It seems currently this is done in the Mix.Tasks.Format, but it doesn't seem to be a public API to retrieve this?
https:/elixir-lang/elixir/blob/main/lib/mix/lib/mix/tasks/format.ex#L357-L365=

@jonatanklosko
Copy link
Member

Unfortunately it's not trivial, when using Mix.install there's currently no easy way to locate dependencies, because Mix.Project.deps_paths() doesn't work. This would most likely require changes to Elixir, so we can wait until there's a stronger indicator we need this :)

@zachdaniel
Copy link

We're using Livebook for things like Ash tutorials, and for Ash.Resource and friends, locals_without_parens is a pretty important aspect in keeping the code readable. Basically all of our tutorials can't be edited in the Livebook directly because saving it will overwrite the formatting, which takes a few minutes of work to address, and is very easy to accidentally do since it of course feels natural to edit the livebook.

I wouldn't mind if special config needs to be set in the setup to make this work, like
Application.put_env(:livebook, :formatter, import_deps: [:ash, ...])

@jonatanklosko
Copy link
Member

@josevalim perhaps if we made Mix.in_install_dir public (or another specific function), we could get the formatter config from all deps. I will experiment with this later!

@jonatanklosko
Copy link
Member

@zachdaniel we decided to disable formatting on-save altogether (#2605).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants