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

Pinned variables in matches are formatted incorrectly #27

Closed
Nezteb opened this issue May 4, 2023 · 9 comments
Closed

Pinned variables in matches are formatted incorrectly #27

Nezteb opened this issue May 4, 2023 · 9 comments

Comments

@Nezteb
Copy link

Nezteb commented May 4, 2023

Elixir / OTP Version

❯ cat .tool-versions 
elixir 1.14.1-otp-25
erlang 25.1.1

Using elixir-styler version 0.5.2.

Current Behaviour

Example 1:
Screenshot 2023-05-04 at 10 34 29 AM

Which gives:

** (ArgumentError) cannot pipe some_var_ids into ^nil, the :^ operator can only take one argument

If I revert and rerun mix format, there is a different result that is still weird:
Screenshot 2023-05-04 at 10 35 05 AM

Example 2:
Screenshot 2023-05-04 at 10 32 09 AM

Oddly enough, if I revert the file and then rerun mix format, this same code is untouched. 🤔

I'm guessing that the non-deterministic behavior is not intended either, though that's probably a separate issue? 😅

Expected Behaviour

Unsure what should actually happen here, but perhaps pinned variables should be ignored?

@novaugust
Copy link
Contributor

just when you think you've seen all the ways people write ecto queries 😂

Unsure what should actually happen here, but perhaps pinned variables should be ignored?

yeah, i'll add pin as a valid start as well

@Nezteb
Copy link
Author

Nezteb commented May 4, 2023

just when you think you've seen all the ways people write ecto queries 😂

Yeah I find stuff like this relatively often. 😅

@novaugust
Copy link
Contributor

if it makes your life easier, all i ever need is the snippet that does weird things for you, to save you the work of blacking out screenshots. can just write
"Heyyyy this snippet breaks things"

^foo
|> Ecto.Query.blah(...)

@novaugust
Copy link
Contributor

want to just point at latest main here on GH until you stop finding issues? i can cut a release after that ^.^

@Nezteb
Copy link
Author

Nezteb commented May 4, 2023

Alright, I've tested main on three large work-related Elixir services, and everything is good so far!

There were a few times where a format would cause code to not compile or tests to fail, but each of those cases were actually good opportunities for refactoring that I was able to fix!

I look forward to trying this as more of the items from #18 are added. 😄

@novaugust
Copy link
Contributor

There were a few times where a format would cause code to not compile or tests to fail, but each of those cases were actually good opportunities for refactoring that I was able to fix!

The only thing I'm aware of breaking compilation is when there are alias before other directives that are then used in them. did you see anything apart from that? (i'll be updating the readme to let people know what to expect on a first run)

@Nezteb
Copy link
Author

Nezteb commented May 4, 2023

when there are alias before other directives that are then used in them. did you see anything apart from that?

Nope! Just stuff like:

alias Some.Long.Path
use Path.Thing

And even worse: (this was technically only a compiler warning, but still)

@some_module_attribute :thing
use Some.Weird.Macro # depends on @some_module_attribute being defined

So now I've changed those to:

use Some.Long.Path.Thing
alias Some.Long.Path

And:

# using actual __using__ opts instead of module attributes
use Some.Weird.Macro,
  some_module_attribute: :thing

😄

@novaugust
Copy link
Contributor

yep, that's 100% the problems i see on first run as well. thanks!

@novaugust
Copy link
Contributor

thanks again for the issues & feedback. kicked out 0.6.0

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

2 participants