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

Added SQL Formatter tool #274

Merged
merged 5 commits into from
Jan 29, 2022
Merged

Added SQL Formatter tool #274

merged 5 commits into from
Jan 29, 2022

Conversation

veler
Copy link
Collaborator

@veler veler commented Jan 27, 2022

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Internationalization and localization
  • Other (please describe):

What is the current behavior?

As a developer working with database, I'd like to be able to format some queries without having to paste them on a random website.

Issue Number: #136

What is the new behavior?

Added a SQL Formatter based on https://github.com/zeroturnaround/sql-formatter

It isn't perfect though. This is a literal translation of the JS library. Some optimizations can be made to reduce the allocation of strings.

Other information

image

image

image

image

Quality check

Before creating this PR, have you:

  • Followed the code style guideline as described in CONTRIBUTING.md
  • Verified that the change work in Release build configuration
  • Checked all unit tests pass

@veler veler requested a review from btiteux January 27, 2022 02:21
@veler veler linked an issue Jan 27, 2022 that may be closed by this pull request
@veler
Copy link
Collaborator Author

veler commented Jan 27, 2022

@akarboush, please feel free to take a look at it :)

@akarboush
Copy link
Contributor

@veler thanks for the mention.
I took a brief look at it, and It looks similar to mine since it's a direct translation.
As you already mentioned, there's a lot of optimization to do (string allocation and some regex could be replaced with a plain C# methods too).

For the mean time, I'll try to optimize it on my branch and then propose the changes if there aren't any here yet

_index = i;

Token token = TokenOverride(_tokens[i]);
if (token.Type == TokenType.LineComment)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could clean a bit all the if else's by using pattern matching.

@veler
Copy link
Collaborator Author

veler commented Jan 27, 2022

@veler thanks for the mention. I took a brief look at it, and It looks similar to mine since it's a direct translation. As you already mentioned, there's a lot of optimization to do (string allocation and some regex could be replaced with a plain C# methods too).

For the mean time, I'll try to optimize it on my branch and then propose the changes if there aren't any here yet

Sounds good. I think I will do a little bit of clean up before merging, but likely no optimization yet. While all these regex and strings are suboptimal, it seems to work relatively well as long as the SQL query isn't super long.

I'd suggest to do the optimizations in a separated PR, what do you think @akarboush ?

@veler veler changed the title Sqlformatter Added SQL Formatter tool Jan 27, 2022
@veler veler merged commit 7ddeb1d into main Jan 29, 2022
@veler veler deleted the sqlformatter branch January 29, 2022 04:41
veler added a commit that referenced this pull request Mar 31, 2023
* Added SQL Formatter tool

* added resw

* Updated terms
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.

SQL formatter
3 participants