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

feat: scarb fmt option to target a specific file. #1534

Closed
enitrat opened this issue Aug 22, 2024 · 8 comments · Fixed by #1626
Closed

feat: scarb fmt option to target a specific file. #1534

enitrat opened this issue Aug 22, 2024 · 8 comments · Fixed by #1626
Assignees

Comments

@enitrat
Copy link
Contributor

enitrat commented Aug 22, 2024

Problem

Describe the Feature Request

Following starkware-libs/cairo#4134, for integration with external tools, cairo-format should have an option to take a specific file as a target, instead of formatting the entire package all at once.

For reference, with the old CairoZero cairo-format, we could do cairo-format ${target} which would format one specific file.
Forge (foundry) supports forge fmt ${target} --check as well. This works great for integration with lint aggregators.

Describe Preferred Solution

Add an optional target argument, referring to the file to format. The command scarb fmt ${target} -e stdout should be valid. If a target is specfied, the name of the file being formatted is not part of the stdout.

cairo-format actually supports this; but scarb fmt doesn't: see below

❯ cargo run --bin cairo-format -- --help
Formats a file or directory with the Cairo formatter.
Exits with 0/1 if the input is formatted correctly/incorrectly.

Usage: cairo-format [OPTIONS] [FILES]...

Arguments:
  [FILES]...  A list of files and directories to format. Use "-" for stdin

Options:
  -c, --check                 Check mode, don't write the formatted files, just output the diff between the original and the formatted file
  -r, --recursive             Format directories content recursively
  -v, --verbose               Print verbose output
  -p, --print-parsing-errors  Print parsing errors
  -s, --sort-mod-level-items  Enable sorting the module level items (imports, mod definitions...)
  -h, --help                  Print help
  -V, --version               Print version

However it's not shown when doing scarb fmt --help

Usage: scarb fmt [OPTIONS]

Options:
-c, --check                  Only check if files are formatted, do not write the changes to disk
-e, --emit <EMIT>            Emit the formatted file to stdout [possible values: stdout]
-v, --verbose...             Increase logging verbosity.
    --no-color               Do not color output
-q, --quiet...               Decrease logging verbosity.
-p, --package <SPEC>         Packages to run this command on, can be a concrete package name (`foobar`) or a prefix glob (`foo*`) [env: SCARB_PACKAGES_FILTER=] [default: *]
    --verbosity <VERBOSITY>  Set UI verbosity level by name. [env: SCARB_UI_VERBOSITY=]
-w, --workspace              Run for all packages in the workspace
-h, --help                   Print help

Delegating this to the scarb team

Proposed Solution

Either modify scarb fmt to accept a specific file path OR expose cairo-format through scarb (like scarb cairo-run does)

Notes

No response

@ShantelPeters
Copy link

Please I will love to work on this

@maciektr
Copy link
Contributor

@ShantelPeters assigning you then :D Good luck!

@ShantelPeters
Copy link

Thank you ser @maciektr i will get straight to work

@enitrat
Copy link
Contributor Author

enitrat commented Aug 26, 2024

@maciektr is scarb fmt just a wrapper around cairo-format (like cairo-run) or is there a more complex logic to implement ?

@maciektr
Copy link
Contributor

maciektr commented Aug 26, 2024

I'd say more complex. It's not a separate binary if that's what you mean, but Scarb built in. The implementation mainly lies here https://github.com/software-mansion/scarb/blob/main/scarb/src/ops/fmt.rs and is mainly based on cairo_lang_formatter::CairoFormatter. Let me know in case you want to hear more :)

@maciektr
Copy link
Contributor

maciektr commented Sep 2, 2024

Hi @ShantelPeters !
How is it going? Do you need any help?

@ShantelPeters ShantelPeters removed their assignment Sep 2, 2024
@enitrat
Copy link
Contributor Author

enitrat commented Sep 2, 2024

@maciektr I might take this later in the week unless you guys have some spare time ;p

@enitrat
Copy link
Contributor Author

enitrat commented Oct 2, 2024

Finally took the time to implement this. Let me know how it is.

github-merge-queue bot pushed a commit that referenced this issue Oct 3, 2024
Allows formatting a single file. Useful for integration with external
tools.

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

Successfully merging a pull request may close this issue.

3 participants