getgrit / gritql

GritQL is a query language for searching, linting, and modifying code.
https://docs.grit.io/
MIT License
3.17k stars 82 forks source link

`grit format` command #574

Open morgante opened 1 week ago

morgante commented 1 week ago

Now that biome has added support for Grit as a formattable language we should add a grit format command to the Grit CLI. We can (hopefully) depend on the Biome crate for this.

grit format should:

Use https://github.com/getgrit/stdlib for a repo to test on, but add at least one CLI test case in https://github.com/getgrit/gritql/tree/main/crates/cli_bin/tests

morgante commented 1 week ago

/bounty $200

algora-pbc[bot] commented 1 week ago

💎 $200 bounty • Grit

Steps to solve:

  1. Start working: Comment /attempt #574 with your implementation plan
  2. Submit work: Create a pull request including /claim #574 in the PR body to claim the bounty
  3. Receive payment: 100% of the bounty is received 2-5 days post-reward. Make sure you are eligible for payouts

Thank you for contributing to getgrit/gritql!

Add a bounty • Share on socials

Attempt Started (GMT+0) Solution
🟢 @harshtech123 Nov 13, 2024, 8:43:59 AM WIP
🟢 @vishwamartur Nov 18, 2024, 7:30:23 AM #580
tareknaser commented 1 week ago

Hello I’d like to take this on. Could I get assigned?

morgante commented 1 week ago

I do not assign issues to first-time contributors. You're welcome to attempt it.

harshtech123 commented 1 week ago

/attempt #574

tareknaser commented 1 week ago

I ran into an issue while implementing this. Is there a way to list only the grit patterns in the current directory? It looks like grit list always includes the standard library workflows too. After tracing the code, I noticed that the _stdlib_workflows boolean parameter in the list_applyables function (located in crates/cli/src/commands/list.rs) is being completely ignored.

morgante commented 1 week ago

Look at how grit patterns test works.

tareknaser commented 1 week ago

Thanks. I updated the code to follow a similar pattern to what's in grit patterns test, and it's working as expected.

I've made progress on this but ran into a blocker, and I’d appreciate your input.

The formatter is working locally by depending on the biome crates, and I verified it by running it on examples here, which worked as expected. I’ve also set up the diff and apply functionality if write is set to true, so that part is sorted.

However, the issue is that the formatter doesn’t actually support the files we want to format. Specifically, it doesn’t support markdown patterns and .grit files with the format seen in the stdlib (e.g., common.grit).

For instance, running the biome formatter on .grit/patterns/go/common.grit fails.

Am I missing something here?

morgante commented 1 week ago

Specifically, it doesn’t support markdown patterns

This is expected. You need to extract the gritql from the markdown file and format the extracted gritql then write it back in. We already have logic (in gritmodules) that handles extracting patterns from markdown.

For instance, running the biome formatter on .grit/patterns/go/common.grit fails.

What fails? We should patch that upstream.

tareknaser commented 1 week ago

This is expected. You need to extract the gritql from the markdown file and format the extracted gritql then write it back in.

This seems like a special case we could address in a follow-up PR. I have the main functionality set up locally, including the grit format command with a write flag, and it scans the current directory as expected.

Do you think I should open a PR for this?

What fails?

It’s a bit inconsistent at the moment—it works for some .grit files, but not all. For example, .grit/patterns/go/go_imports.grit fails for me. Here’s the error I’m getting:

---- format_write stdout ----
thread 'main' panicked at /Users/tareknasser/.cargo/git/checkouts/biome-d49ce8898b420a50/8a90b89/crates/biome_parser/src/diagnostic.rs:350:14:
Expected token to be a punctuation or keyword.
morgante commented 1 week ago

This seems like a special case we could address in a follow-up PR. I have the main functionality set up locally, including the grit format command with a write flag, and it scans the current directory as expected.

It's not a special case. I called it out as core functionality that is intrinsic to this ticket.

Do you think I should open a PR for this?

You can, but the bounty will not be awarded until the issue is completed. You must handle .md and .yaml files, as stated in the spec.

It’s a bit inconsistent at the moment—it works for some .grit files, but not all.

That's a biome issue, so you can ignore it. It's out of scope for this issue.

vishwamartur commented 4 days ago

/attempt #574

algora-pbc[bot] commented 4 days ago

💡 @vishwamartur submitted a pull request that claims the bounty. You can visit your bounty board to reward.