Skip to content

Add formatter program to amber-lsp#91

Draft
tye-exe wants to merge 39 commits intoamber-lang:mainfrom
tye-exe:formatting
Draft

Add formatter program to amber-lsp#91
tye-exe wants to merge 39 commits intoamber-lang:mainfrom
tye-exe:formatting

Conversation

@tye-exe
Copy link
Contributor

@tye-exe tye-exe commented Oct 31, 2025

I'm opening this here so I can document the changes I make and get answers to any questions that arise in a centralised place.

This follows on from #18.

This Is A Work In Progress

Any and all structures, names, functions, and anything you can think of is liable to change due to feedback or my own development process as I gain a better understanding of amber-lsp.

@tye-exe
Copy link
Contributor Author

tye-exe commented Oct 31, 2025

@KrosFire Would it be better for me to finalise splitting up the lsp into multiple crates and merge that before fully starting on the formatter change?
This would make it easier for me to merge upstream changes, as the structure would be shared.

@tye-exe tye-exe changed the title Add seperate formatting binary to amber-lsp Add formatter program to amber-lsp Oct 31, 2025
@KrosFire
Copy link
Member

@KrosFire Would it be better for me to finalise splitting up the lsp into multiple crates and merge that before fully starting on the formatter change?

This would make it easier for me to merge upstream changes, as the structure would be shared.

Yes, I think the restructure should be reviewed first. It will be easier to see the changes

@tye-exe
Copy link
Contributor Author

tye-exe commented Nov 5, 2025

I don't know if there is much more restructuring for me to do over what I have already done, as I won't be touching much of the parsing and LSP code. If the LSP specific code is able to be separated from the more general parsing code, then that would be ideal.

I can have a look at trying to separate the LSP code from the main library, but it will take me more time.

This is for my development and not a fully planned part of the change
This only has basic support for some text current.
This is to gather feedback on approaches and methodology
@KrosFire
Copy link
Member

KrosFire commented Nov 6, 2025

I don't know if there is much more restructuring for me to do over what I have already done, as I won't be touching much of the parsing and LSP code. If the LSP specific code is able to be separated from the more general parsing code, then that would be ideal.

I can have a look at trying to separate the LSP code from the main library, but it will take me more time.

I was thinking of that - moving grammar/analysis/types to separate crates in workspace. It should improve compile speeds (they're killing me).

I understand if it's too much for a start, I can take it for a spin next week

@tye-exe
Copy link
Contributor Author

tye-exe commented Nov 9, 2025

Me trying to split up everything even more would be a bit too much work, as I am quite busy currently.

It should be easy enough for me to rebase my changes once the codebase has been split up more, since they are mostly self contained.

@KrosFire
Copy link
Member

KrosFire commented Nov 9, 2025

Me trying to split up everything even more would be a bit too much work, as I am quite busy currently.

It should be easy enough for me to rebase my changes once the codebase has been split up more, since they are mostly self contained.

Okay, focus on formatter. I'm implementing some big changes right now, so i'll split up everything when the dust has settled

@coderabbitai
Copy link

coderabbitai bot commented Feb 22, 2026

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

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

Successfully merging this pull request may close these issues.

2 participants