language-tools icon indicating copy to clipboard operation
language-tools copied to clipboard

feat: add prisma format functionality

Open mcnaveen opened this issue 1 year ago • 10 comments

  • This PR adds format option to the Prisma language tool when the schema.prisma file is opened.

[!WARNING]
Untested code. Looking for someone to test it locally. I'm unsure about running and building it. But I added the functionality. Hope this works or Happy to fix and Improve it.

mcnaveen avatar Aug 09 '24 15:08 mcnaveen

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Aug 09 '24 15:08 CLAassistant

I am looking for someone to test it locally. If you're interested. Please contribute. Thank you. 🙏

mcnaveen avatar Aug 09 '24 15:08 mcnaveen

Hello @mcnaveen,

First of all, thanks for your contribution and willingness to collaborate in making prisma better!

Have you tried using the built-in format option in VSCode? Language formatting via the Prisma’s extension formatting tool is automatically invoked by VSCode when running the “Format Document” command from the taskbar. I think that should serve the purpose to what you're trying to achieve here.

https://github.com/user-attachments/assets/a9cdcf95-cab0-4f7a-b8fb-fa46fbe46afa

apolanc avatar Aug 20 '24 16:08 apolanc

Hi @apolanc Thanks for giving feedback on this PR.

You're making a great point on the VSCode format option and I agree with that. But still, there is a functionality prisma format does is auto mapping the relations between models.

Let's say you have two models User and Posts where the user belongs to multiple posts/owns multiple posts. In that case, you have a userId field in the Posts table/model that refers to the Id of the User table.

Running the prisma format will make sure to reference it in the Users table if it's not already mapped. This won't work with the VSCode feature you're mentioning in the above video.

I hope you're getting my point. If it doesn't make sense. let me know, I'll record a video.

Thank you.

mcnaveen avatar Aug 20 '24 18:08 mcnaveen

@mcnaveen thanks for the answer. Please record a short video to showcase what you had in mind here :)

I'll keep this contribution PR open until we get the capacity to review it.

Thanks once again!

apolanc avatar Aug 21 '24 15:08 apolanc

Hi @apolanc I didn't realize it works with the VSCode format option.

https://github.com/user-attachments/assets/2b8bb66f-7576-4096-9f64-7394cd7af241

mcnaveen avatar Aug 21 '24 15:08 mcnaveen

Hi @apolanc I didn't realize it works with the VSCode format option.

Help me here a bit, would you still see this PR as beneficial or do you realize it's not necessary anymore? :)

apolanc avatar Aug 21 '24 16:08 apolanc

@mcnaveen thoughts here? if none, I'll close this PR to not keep it hanging.

apolanc avatar Aug 23 '24 07:08 apolanc

Hi @apolanc Yes, it's a nice option to have in terms of DX. It can be merged!

mcnaveen avatar Aug 23 '24 07:08 mcnaveen

Thanks for the answer. I'll add this in our list of community contributions and an Engineer should be able to review and evaluate the solution. We will then decide what will happen in the PR or if we need anything from you, we'll let you know.

Thanks once again for contributing!

apolanc avatar Aug 26 '24 08:08 apolanc

Hey @mcnaveen! Thanks for the PR and sorry for the delay in response. While this is an interesting idea, I don't believe formatting should be available as a code lens, this is superfluous and inconsistent with how other language servers and language-specific plugins work. We provide the formatting functionality via the language server protocol, so you can format your Prisma schema exactly the same way as when editing other file types: via the command palette or application menu, using the keybindings (⇧⌥F / ⌘K ⌘F by default in VS Code on macOS) or by turning on autoformatting on save.

aqrln avatar Nov 30 '24 11:11 aqrln

Hi @aqrln, Thank you for the response. It makes sense that we should also follow best practices to be consistent with other tooling.

mcnaveen avatar Nov 30 '24 11:11 mcnaveen