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

VSCodeの設定UIからurborosql-fmtのオプションを指定できるようにする #52

Merged
merged 10 commits into from
May 2, 2024

Conversation

abcd-ts
Copy link
Collaborator

@abcd-ts abcd-ts commented Apr 30, 2024

Close #19

概要

  1. settings.jsonでuroborosql-fmtの設定を指定できるようにしました。
  2. settings.jsonで指定したオプションが設定ファイル (.uroborosqlfmtrc.json) のオプションが競合した場合は、settings.jsonの値が優先されます (Add an api that gives higher priority options uroborosql-fmt#34)

できていないこと

settings.jsonの内容を設定ファイルに同期するコマンドは未実装です

@abcd-ts abcd-ts requested a review from ota-meshi April 30, 2024 06:53
Copy link

changeset-bot bot commented Apr 30, 2024

🦋 Changeset detected

Latest commit: cd43029

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
uroborosql-fmt Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@abcd-ts abcd-ts changed the title Draft: VSCodeno Draft: VSCodeの設定UIからurborosql-fmtのオプションを指定できるようにする Apr 30, 2024
@abcd-ts
Copy link
Collaborator Author

abcd-ts commented Apr 30, 2024

@ota-meshi
settings.jsonでオプションを指定するPRを作成しました。
以下2点の確認をお願いしたいです。

  1. 不自然なコードがないか
  2. settings.jsonの内容を設定ファイルに同期するコマンドをこのPRで実装するか (実装しないようなら、Draft: を外します)

追記

true または false で指定するオプションについて、設定ファイルよりもsettings.jsonの内容を優先する関係上、型を["boolean", "null"] としています。その影響でUI上では settings.jsonで編集 と表示され、チェックボックスで指定することができません。良い案があれば教えていただきたいです。

image


// translate null (that means unsupecified option) to undefined
const removedNullSettings = Object.fromEntries(
Object.entries(restConfiguration).map(([key, value]) => [
Copy link
Member

Choose a reason for hiding this comment

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

undefinedに置き換えてもkey自体は除外されないので、filter使う方が良さそうです。

Suggested change
Object.entries(restConfiguration).map(([key, value]) => [
Object.entries(restConfiguration).filter(([, value]) => value !== null)

@ota-meshi
Copy link
Member

settings.jsonの内容を設定ファイルに同期するコマンドをこのPRで実装するか (実装しないようなら、Draft: を外します)

コマンドはPRを別にしたほうが、PR一つがシンプルになるので良いことだと思います 😄

@ota-meshi
Copy link
Member

ota-meshi commented Apr 30, 2024

true または false で指定するオプションについて、設定ファイルよりもsettings.jsonの内容を優先する関係上、型を["boolean", "null"] としています。その影響でUI上では settings.jsonで編集 と表示され、チェックボックスで指定することができません。良い案があれば教えていただきたいです。

ここらへん詳しく無いのですが、type: booleanにしてdefaultを削除するのはうまくいかないのでしょうか?
その場合、設定なしの場合に true、 false 以外(つまりnullやundefined)が取得できてほしいということですよね?

@ota-meshi
Copy link
Member

type: booleanにしてdefaultを削除するのはうまくいかないのでしょうか?

試しましたがダメそうですね 😓
そうすると(変換処理が必要になりますが)以下のような設定にするのはどうでしょうか?

        "uroborosql-fmt.complementAlias": {
          "enum": [
            "on",
            "off",
            "default"
          ],
          "default": "default",
        },

@ota-meshi
Copy link
Member

(変換処理が必要になりますが

変換が不要な方法だとこうすることもできることに気がつきました。

          "enum": [
            true,
            false,
            null
          ],
          "default": null,

package.json Outdated
},
"uroborosql-fmt.tabSize": {
"type": [
"number",
Copy link
Member

Choose a reason for hiding this comment

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

ここって"integer"って使えませんでしたっけ?あとminimumも指定があるとより良いと思います。

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ありがとうございます。以下対応しました

  1. booleanのオプションについて、enumを使ってUI上で選択できるよう修正しました
  2. tabSizeについて、 typeを integer に変更し、最小値を0に設定しました

@abcd-ts abcd-ts changed the title Draft: VSCodeの設定UIからurborosql-fmtのオプションを指定できるようにする VSCodeの設定UIからurborosql-fmtのオプションを指定できるようにする May 1, 2024
@abcd-ts abcd-ts force-pushed the dev-saito-improve-settings-ui branch from 8e65a0c to 9a2c02b Compare May 2, 2024 00:51
Copy link
Member

@ota-meshi ota-meshi left a comment

Choose a reason for hiding this comment

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

変更ありがとうございます!

package.json Outdated
"number",
"null"
],
"enum": [
Copy link
Member

Choose a reason for hiding this comment

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

ここはnumberなのでenumいらない気がします。

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

見落としていました、ありがとうございます。修正しておきました。

また、ここも整数でよさそうなので、numberからintegerに変更しました。

Copy link
Member

@ota-meshi ota-meshi left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you!

@ota-meshi ota-meshi merged commit ee4d6d4 into main May 2, 2024
2 checks passed
@ota-meshi ota-meshi deleted the dev-saito-improve-settings-ui branch May 2, 2024 01:49
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.

vscodeの設定UIでuroborosql-fmtの設定出来るようにする
2 participants