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

schema change for database section #79

Closed
gilesknap opened this issue Jul 3, 2023 · 6 comments
Closed

schema change for database section #79

gilesknap opened this issue Jul 3, 2023 · 6 comments

Comments

@gilesknap
Copy link
Member

gilesknap commented Jul 3, 2023

At present we have

databases:
      - file: XXX.template
        include_args: [ A, B, D ]
        define_args: |
          C={{ xxx }}
          E={{ yyy }}

First, the use of array/multiline string for include/define is inconsistent.
Second, Ordering of arguments is lost because they are distributed to two places in the YAML.

I propose this approach:

databases:
      - file: XXX.template
        args:  A B C={{ xxx }} D E={{ yyy }} 

Whitspace is the separator (except inside jinja quotes) so that you can format as needed e.g.

databases:
      - file: XXX.template
        args:  |
          A B D
          C={{ xxx }}
          E={{ yyy }} 

@coretl @niamhdougan any thoughts?

@coretl
Copy link
Contributor

coretl commented Jul 3, 2023

Slight concern that we are recreating YAML parsing inside a string... At the cost of some vertical space we could format as a list:

databases:
  - file: XXX.template
    args:
      - A
      - B
      - C={{ xxx }}
      - D
      - E={{ yyy }}

or a dict:

databases:
  - file: XXX.template
    args:
      A:
      B:
      C: "{{ xxx }}"
      D:
      E: "{{ yyy }}"

@coretl
Copy link
Contributor

coretl commented Jul 3, 2023

Actually, the inline version of the dict is reasonable:

databases:
  - file: XXX.template
    args: { A, B, C: "{{ xxx }}", D, E: "{{ yyy }}" }

@coretl
Copy link
Contributor

coretl commented Jul 3, 2023

args outputs

{'A': None, 'B': None, 'C': '{{ xxx }}', 'D': None, 'E': '{{ yyy }}'}

@gilesknap
Copy link
Member Author

I quite like that.

vscode will format it vertically once the length exceeds 80 chars.

So we get no say in what the layout is but that is probably OK.

@gilesknap
Copy link
Member Author

@coretl

Maybe its worth mentioning that this results in the following format for longer lists of args. ALSO the auto generator will always use this form (because ruamel)

    databases:
      - file: $(PMAC)/db/geobrickette_motion_stop.template
        args:
          active_DESC:
          mstop_ZNAM:
          active_OSV:
          active_VAR:
          SCAN:
          mstop_ZSV:
          P:
          mstop_R:
          active_ONAM:
          active_ZNAM:
          mstop_DESC:
          mstop_VAR:
          active_R:
          TIMEOUT:
          active_ZSV:
          mstop_ONAM:
          PORT:
          mstop_OSV:\

@gilesknap
Copy link
Member Author

sorry above not quite true. The above is what ruamel will do. Vscode formatting will do:

    databases:
      - file: $(PMAC)/db/pmacVariableReadLED.template
        args:
          {
            ZSV,
            name,
            OSV,
            ZNAM,
            ONAM,
            SPEED,
            LABEL,
            VAR,
            PORT,
            P,
            Q,
          }

gilesknap added a commit that referenced this issue Jul 6, 2023
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

No branches or pull requests

2 participants