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

SQL formatting options #16

Closed
frigus02 opened this issue Dec 2, 2020 · 8 comments · Fixed by #19
Closed

SQL formatting options #16

frigus02 opened this issue Dec 2, 2020 · 8 comments · Fixed by #19
Labels
enhancement New feature or request

Comments

@frigus02
Copy link
Owner

frigus02 commented Dec 2, 2020

So amazing, really impressive work @frigus02 !! 🔥

Just giving it a spin, seems to be working!! I'm going to need to probably pass some additional configuration options to pgFormatter such as --comma-break (see screenshots below), but I suppose that will require:

  1. Passing the option(s) along through gajus/pg-formatter, à la your PR at Add "tabs" and "noRcFile" options gajus/pg-formatter#7
  2. Setting this option in the typescript-sql-tagged-template-plugin and vscode-sql-tagged-template-literals (wonder if --comma-break should be even the default)

INSERT without --comma-break

Screen Shot 2020-11-28 at 16 32 09

INSERT with --comma-break

Screen Shot 2020-11-28 at 16 34 18

Screenshot source: http://sqlformat.darold.net/

Originally posted by @karlhorky in #4 (comment)

@frigus02
Copy link
Owner Author

frigus02 commented Dec 2, 2020

Sorry for the delayed response @karlhorky. I'm moving home and the last few days have been super busy. I moved this into a new issue to make this more visible.

I agree that supporting all the different formatting options would be nice.

I see 2 different ways to do that:

  1. Add every setting as a setting to this pg-formatter, this plugin and the extension. This is nice the users but possibly quite a bit of maintenance effort.
  2. Utilize the config file support. We could add 1 option: --config FILE to pg-formatter and this plugin. The extension could possibly find a .pg_format file in the workspace root automatically and pass it in. This has the benefit that it automatically supports all pgFormatter options, including future options. But it's possibly slightly less convenient.

I think I would prefer the config file option or something that works similarly.

What do you think?

@frigus02 frigus02 added the enhancement New feature or request label Dec 2, 2020
@karlhorky
Copy link

No worries at all! You've done amazing work. I hope that your move is going well!

Config file seems reasonable, sure! Through the single file (seems like this is the format), it would support all options in one go, and have no maintenance effort in the future. Can probably close gajus/pg-formatter#8 if this is a good way forward.

@frigus02
Copy link
Owner Author

frigus02 commented Jan 2, 2021

Hi @karlhorky. I finally found some time to work on this. I published v0.1.1 of this package and v0.0.17 of the corresponding VS Code extension.

They now allow you to specify a pg_format config file, which means you have all pgFormatter options available, as seen in pg_format.conf.sample. You probably want to stay away from some of them like format=html 😄. I tested comma=start, comma-break=1 and keyword-case and they worked perfectly.

I hope this works for you as well 🤞

@karlhorky
Copy link

Hey @frigus02 , Happy New Year! 🎉✨ Hope the move went well and the year is starting off nicely!

Really awesome work! 🙌 💯 Looks great - custom implementation of the pgFormatter, nice! 🤯

@karlhorky
Copy link

karlhorky commented Jan 2, 2021

Now that all features are there and this is looking good, I've been thinking about how to enable this for our @upleveled students. At first I thought that I would configure the TypeScript and JavaScript Language Features as a second formatter for the students, but there are some downsides to this:

So I thought of another approach - why not get the students to manually run the formatter on the command line once at the end, when they are ready to submit their projects? I can use this opportunity to fix other things / check for errors too, such as using ESLint with our eslint-config-upleveled.

So now a new interesting problem - how to use this TypeScript Language Service plugin on the command line?

Since this was long and could result in a discussion that may be helpful for others, moved this to a new issue: #20

@frigus02
Copy link
Owner Author

frigus02 commented Jan 2, 2021

Yeah, it would be amazing if formatting could be integrated with Prettier. But I think this would be out of scope for this plugin.

@karlhorky
Copy link

But I think this would be out of scope for this plugin

Definitely, for sure! This plugin is about extending the TypeScript Language Server, which makes sense for the plugin that is supporting a VS Code Extension.

There are efforts such as https://github.com/benjie/prettier-plugin-pg by @benjie to create a Prettier plugin, but I think the concerns are different enough (eg. the Prettier-specific AST) that the two approaches wouldn't have much overlap.

@benjie
Copy link

benjie commented Jan 3, 2021

Prettier can use any AST I think (though some might need to be transformed a little); the main issue holding up prettier-plugin-pg is that comments are dropped by postgres' parser, and the postgres parser does not contain enough information about the location of the tokens that comments can be restored into the correct places even if we were to write a separate parallel parser just for comments. We need an alternative parser for PostgreSQL and I've not yet found one that I feel covers sufficient of Postgres' features without introducing issues (and a code formatter should never break your code!). I'm interested in this though; maybe it'll be useful for pg-sql2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants