Feat: allow configuring default formatting behavior in the UI via sql…

[z3z1ma]

…mesh config. In subsequent iterations perhaps this serves as the "default options" while a modal allows overriding them for the session. But as a general source of truth, I think its better here since you have consistency across the entire team as well as version control.

default_gateway: duckdb
gateways:
  duckdb:
    connection:
      type: duckdb

model_defaults:
  dialect: 'duckdb'

format:
  pad: 4
  indent: 4
  leading_commas: true
  max_text_width: 120

ui:
  format_on_save: true

[CLAassistant]

All committers have signed the CLA.

[mykalmax]

should we make format top level, so it works for CLI

default_gateway: duckdb
gateways:
  duckdb:
    connection:
      type: duckdb

model_defaults:
  dialect: 'duckdb'

format:
  pad: 4
  indent: 4
  leading_commas: true
  max_text_width: 120

ui:
  format_on_save: true

@tobymao @izeigerman

[z3z1ma]

I wondered the same, personally I think top level would be best. Then the CLI flags would just serve as overrides.
This would ensure uniformity between both uses of SQLMesh without having to ensure your CLI flags match ui opts.

[izeigerman]

@z3z1ma @mykalmax +1 to making format settings top-level

[z3z1ma]

@izeigerman I cleaned it up a bit, renamed the newline option to make its intent clear, and made sure format_options is a top level config applicable to both the CLI and UI, and lastly that CLI flags take precedence over the config settings as would be expected.

[z3z1ma]

Seems like a pydantic inconsistency on v1?

[mykalmax]

do we actually need an option to override global format by passing flags to CLI, every override will be adding number of changes to the project repo and maybe be overridden by someone running format without override flags later

should we then do the same for ui part for config ?
(bringing back)

ui:
  format_on_save: true
  format_options:
    pad: 4
    indent: 4
    leading_commas: true
    max_text_width: 120

[z3z1ma]

@mykalmax I think the user-specific config in the home directory supported by sqlmesh will already solve for user specific overrides in the ui technically. i think longer term, there should be a modal in the UI with options preset based on the format config. And they can tweak there in theory.

[z3z1ma]

I also think CLI precedence is typical enough to expect in a tool, it allows for faster iteration -- or imperative flows which dont use the config.py/yaml. I dont think its necessary to constrain it personally 🤷

[z3z1ma]

Thinking about it more @mykalmax ,
I could go either way really on dropping the cli flags and using just the yaml honestly

Since there is some simplification in the code then. And is easier to grok than stacking CLI flags. And its uniform with UI by default as you said.

[mykalmax]

@z3z1ma would it make more sense to have format top level with enable: true as default (and false to disable formatting), then drop ui from config and if format is set for a project format-on-save on ui just be reading options from config.
Remove all overrides (flags), so only way to change/set format is to change it in config for entire project (or disable).
In this case if projects needs formatting: pass options to config and it will be consistent in cli and ui and if formatting not needed , just disable for entire project.
@tobymao @izeigerman

[z3z1ma]

I think thats fine to consolidate and drop flags if everyone agrees @mykalmax

I would maybe nest the ui format-on-save option though.

format:
  pad: 4
  indent: 4
  normalize: false
  ui:
    enabled: true # auto format on save

I think being able to run sqlmesh format in the CLI and the UI auto-formatting files on save are 2 different concerns and the one worth disabling is the uncontrollable implicit one (the UI). The CLI is by contrast a very explicit action and a user running it, typically wants to run it. And if they dont want to run it, they dont run it. We don't need to make it a no-op for the sake of consistency with UI autoformat.

I will await decision. (can always pivot later 😄)

[mykalmax]

@izeigerman @z3z1ma my suggestion was to drop ui: from config at all and if formatting is enabled for a project UI just picks up settings from format: in config.
But with option to opt-out from auto-formatting on save on UI it should be separate top level ui: to avoid nesting in this case and to group other UI related options.
Sorry for confusion

...

format:
  pad: 4
  indent: 4
  leading_commas: true
  max_text_width: 120

ui:
  format_on_save: true

...

[z3z1ma]

@izeigerman can you retrigger test. Seems to be hanging? Or I can do another commit I guess

[izeigerman]

Looks great! Thanks a lot for addressing comments 👍