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

📋 Formatter: Syntax Formatting #4798

Closed
65 of 72 tasks
MichaReiser opened this issue Jun 2, 2023 · 16 comments
Closed
65 of 72 tasks

📋 Formatter: Syntax Formatting #4798

MichaReiser opened this issue Jun 2, 2023 · 16 comments
Assignees
Labels
formatter Related to the formatter help wanted Contributions especially welcome

Comments

@MichaReiser
Copy link
Member

MichaReiser commented Jun 2, 2023

This task coordinates the work on implementing formatting for the Python syntax.

Please create an issue and comment that you intend to work on before starting a new syntax. This allows us to better coordinate the work.

Module

Note: Our infrastructure only supports testing Module. We should, therefore, wait to implement the other variants.

Statements

Expressions

Improvements

Cross-cutting

Bugs

See open issues

Notes

#4730 removed the prototype from @charliermarsh. It's worth taking a look at it when implementing new formatting because it already formatted a lot of syntaxes correctly.

@MichaReiser MichaReiser added the formatter Related to the formatter label Jun 2, 2023
@JonathanPlasse
Copy link
Contributor

Alilas?

@MichaReiser
Copy link
Member Author

Alilas?

It seems to be used in ImportFrom. I moved it to ImportFrom.

@MichaReiser
Copy link
Member Author

MichaReiser commented Jun 5, 2023

If someone's interested in familiarizing themselves with the formatter API, then the following are probably good starters:

  • Pass, Break, and Continue: I don't expect this to change the formatter output (also because we only support formatting while statements) but you can see how the IR and write macro work
  • Constant::None and Bool: Same as for Pass...
  • Constant::Ellipsis: this should be easy, but I believe black removes ellipsis sometimes. It would be good to understand when this is the case. We don't necessarily have to implement it in the first PR
  • nolocal and global
  • These may be good starts, but could also be difficult.
    • UnaryOp
    • Await
    • YieldFrom
    • Return
    • Raise
    • Assert
    • Delete

Let me know if you're interested.

For testing. We have tests that automatically compare our output with black's output. You can run them with cargo test and review the changes with cargo insta review. You can also add your own tests by adding a file to crates/ruff_python_formatter/resources/test/fixtures/ruff (it may sometimes be necessary to run a cargo clean in case your test doesn't get picked up)

What's worth testing:

  • Comments in all possible positions :D

@MichaReiser MichaReiser added the help wanted Contributions especially welcome label Jun 5, 2023
@addisoncrump
Copy link
Contributor

Is the goal here to reach parity with black?

@MichaReiser
Copy link
Member Author

Is the goal here to reach parity with black?

The goal is to be black-compatible except for pathological cases.

@addisoncrump
Copy link
Contributor

Okay. Depending on how strictly you want to be having the exact same output as black, this could potentially be tested with differential fuzzing. If that's desirable, please let me know and I can write a harness for this 🙂

Otherwise, I will likely swing back around once this reaches a steady state and add an idempotency fuzzer.

@warsaw
Copy link

warsaw commented Jun 6, 2023

The goal is to be black-compatible except for pathological cases

I would dearly love at least the option to choose a different default quote character. At least on US keyboards, double quotes are ergonomically inferior. It's the number one reason why blue exists.

@charliermarsh
Copy link
Member

We still plan to make the quote style and indentation style configurable, though the exact settings scheme etc. haven’t been designed yet.

@MichaReiser
Copy link
Member Author

See #1904 for an in-depth discussion around the goal. The purpose of this task is to track the implementation work

This was referenced Jun 9, 2023
@MichaReiser
Copy link
Member Author

Some other good first issues could be

  • Star expression
  • Yield and Yield from

@LouisDISPA
Copy link
Contributor

LouisDISPA commented Jul 7, 2023

Hello, I'm interested in the formatter and while having a look at the code I did the raise formatter code. I'm not sure it's good but I can open a PR if you want. I took inspiration from the return formatter.

@MichaReiser
Copy link
Member Author

Hello, I'm interested in the formatter and while having a look at the code I did the formatter code. I'm not sure it's good but I can open a PR if you want. I took inspiration from the return formatter.

Feel free to open a PR. On which node where you working?

@LouisDISPA
Copy link
Contributor

LouisDISPA commented Jul 7, 2023

Feel free to open a PR. On which node where you working?

Sorry I edited my comment, it was the StmtRaise node.
Ok I'll open the PR!

This was referenced Jul 10, 2023
@cnpryer
Copy link
Contributor

cnpryer commented Jul 15, 2023

Any chance we can open an issue for lambda? I have a lambda branch but I'm not sure I'm ready to claim it. If I have any questions it'd be nice to ask there.

@MichaReiser
Copy link
Member Author

MichaReiser commented Jul 17, 2023

Any chance we can open an issue for lambda? I have a lambda branch but I'm not sure I'm ready to claim it. If I have any questions it'd be nice to ask there.

Sure: #5826

charliermarsh added a commit that referenced this issue Jul 29, 2023
## Summary

Adds `global` and `nonlocal` formatting, without the "deviation from
black" outlined in the linked issue, which I'll do separately.

See: #4798.

## Test Plan

Added a fixture in the Ruff-specific directory since the Black fixtures
don't seem to cover this.
@MichaReiser MichaReiser added this to the Formatter: Alpha milestone Jul 31, 2023
@MichaReiser
Copy link
Member Author

I'm closing this one. There's still some work left to be done but we'll track this as part of #6203

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
formatter Related to the formatter help wanted Contributions especially welcome
Projects
None yet
Development

No branches or pull requests

8 participants