[add] sql-formatter params

[ko-ya346]

What type of PR is this?

• [ ] Refactor
• [x] Feature
• [ ] Bug Fix
• [ ] New Query Runner (Data Source)
• [ ] New Alert Destination
• [ ] Other

Description

This pull request introduces significant improvements to the query editor's formatting functionality in Redash. Previously, the query editor's format feature was limited to default settings. With this update, users can now utilize custom formatting settings defined in a configuration file, allowing for greater flexibility and personalization.

Key Changes:

1. Upgraded sql-formatter and @types/sql-formatter

The versions of sql-formatter and its corresponding TypeScript type definitions have been updated. This upgrade ensures better performance and compatibility with the new formatting features.

2. Loading Custom Settings from Configuration File in queryFormat.js Implemented new functionality in queryFormat.js to read formatting settings from an external configuration file. This allows users to define their preferred formatting options, such as indentation style, keyword casing, and other SQL formatting standards.

This enhancement aims to provide a more user-friendly and customizable experience in the query editor, catering to diverse coding styles and preferences.

How is this tested?

• [ ] Unit tests (pytest, jest)
• [ ] E2E Tests (Cypress)
• [ ] Manually
• [ ] N/A

Related Tickets & Documents

Mobile & Desktop Screenshots/Recordings (if there are UI changes)

[AndrewChubatiuk]

@gabrieldutra sqlformatter supports now advanced configuration, so cases, that are covered in a fork can be easily implemented using a latest library version

[ko-ya346]

@AndrewChubatiuk Thanks to your advice I was able to perform the formatting function without any errors in cases involving query parameters. Thank you very much.

[guidopetri]

@ko-ya346 thanks for the PR! It'd be pretty nice to remove a manually-maintained dependency.

I set the checks to run on this PR, let's see if they pass.

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 63.41%. Comparing base (af0773c) to head (9a6b50a). Report is 33 commits behind head on master.

:exclamation: Current head 9a6b50a differs from pull request most recent head 7471d9a. Consider uploading reports for the commit 7471d9a to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6724      +/-   ##
==========================================
- Coverage   63.82%   63.41%   -0.41%     
==========================================
  Files         161      162       +1     
  Lines       13060    13169     +109     
  Branches     1803     1819      +16     
==========================================
+ Hits         8335     8351      +16     
- Misses       4425     4522      +97     
+ Partials      300      296       -4     

see 21 files with indirect coverage changes

[guidopetri]

@ko-ya346 it looks like the frontend unit tests are failing. Would you mind investigating and seeing if they pass on your local machine?

[AndrewChubatiuk]

looks like this module requires changes in webpack config and its dependencies

[ko-ya346]

@guidopetri @AndrewChubatiuk

I ran fromtend unit test on my local machine and it failed. The stdout from the run is as follows. I will investigate the cause after this, but if you know of a solution I would appreciate it if you could let me know.

$ make frontend-unit-tests
CYPRESS_INSTALL_BINARY=0 PUPPETEER_SKIP_CHROMIUM_DOWNLOAD=1 yarn --frozen-lockfile
yarn install v1.22.19
$ cd viz-lib && yarn link --link-folder ../.yarn
yarn link v1.22.19
warning There's already a package called "@redash/viz" registered. This command has had no effect. If this command was run in another folder with the same name, the other folder is still linked. Please run yarn unlink in the other folder if you want to register this folder.
Done in 0.03s.
[1/5] Validating package.json...
[2/5] Resolving packages...
success Already up-to-date.
$ (cd viz-lib && yarn --frozen-lockfile && yarn build:babel) && yarn link --link-folder ./.yarn @redash/viz
yarn install v1.22.19
[1/4] Resolving packages...
success Already up-to-date.
Done in 0.35s.
yarn run v1.22.19
$ yarn type-gen && yarn build:babel:base
$ tsc --emitDeclarationOnly
$ babel src --out-dir lib --source-maps --ignore 'src/**/*.test.js' --copy-files --no-copy-ignored --extensions .ts,.tsx,.js,.jsx
Browserslist: caniuse-lite is outdated. Please run:
  npx update-browserslist-db@latest
  Why you should do it regularly: https://github.com/browserslist/update-db#readme
Successfully compiled 169 files with Babel (2770ms).
Done in 6.99s.
yarn link v1.22.19
success Using linked package for "@redash/viz".
Done in 0.04s.
Done in 8.28s.
yarn test
yarn run v1.22.19
$ run-s type-check jest
$ tsc --noEmit --project client/tsconfig.json
client/app/lib/queryFormat.ts:2:24 - error TS2307: Cannot find module 'sql-formatter' or its corresponding type declarations.

2 import { format } from "sql-formatter";
                         ~~~~~~~~~~~~~~~


Found 1 error.

error Command failed with exit code 2.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
ERROR: "type-check" exited with 2.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
make: *** [Makefile:45: frontend-unit-tests] エラー 1

[ko-ya346]

The frontend tests now pass on my local machine following the recent commit.

[ko-ya346]

@guidopetri I may have made an incorrect commit, possibly disregarding your review.

[ko-ya346]

The frontend-e2e-tests are failing, although they passed previously. I cannot determine the cause from the logs, so I will attempt to reproduce the issue for further investigation.

[guidopetri]

I'm not sure that the tests failing is related to your changes. Let me investigate first before you spend too much time on a red herring. Do the tests pass for you on your local?

[ko-ya346]

@guidopetri Thank you for your feedback. I ran the commands locally and confirmed that everything passed. It seems that the issue is not related to my code changes.

yarn cypress build
yarn cypress start
yarn cypress run 

[guidopetri]

Thanks @ko-ya346 :) I won't merge this in until our CI on master is passing again, but it does seem like this PR is done. Thank you so much for your work, and hopefully soon we'll merge this in! I'll ping you again if anything else is necessary, but I appreciate all your work.

[AndrewChubatiuk]

Hey @ko-ya346 Could you please rebase your branch?

[AndrewChubatiuk]

Tests have passed! LGTM

[AndrewChubatiuk]

@justinclift could you please merge it?

[ko-ya346]

@guidopetri test passed! could you please merge it?