wireviz / WireViz

Easily document cables and wiring harnesses.
GNU General Public License v3.0
4.35k stars 223 forks source link

[feature] ability to import external yaml files for sharing connector and cable definitions #220

Open gmkado opened 3 years ago

gmkado commented 3 years ago

I would love to be able to have all my connector definitions in a single file and include it into my cable drawing yamls. This would allow for things like a shared "library" of connectors that can be shared across a company.

From my searches online there are ways to do this with pyyaml:

I tried adding this code to the wireviz.py dev build but keep running into yaml syntax errors. My knowledge of yaml parsing is pretty limited.

kvid commented 3 years ago

Thank you for your effort, but please also try including your library using the existing command line option --prepend-file. Does that solve your issue? If not, please describe use cases where more or different features are required.

gmkado commented 3 years ago

oops, should have read the docs better. Thanks this does the trick!

gmkado commented 3 years ago

Hmm I'm going to re-open this because the --prepend-file leaves something to be desired. Ideally you would be able to keep common connector and cable definitions in a separate folder and point to it. But doing this breaks paths to image files referenced by the definitions.

kvid commented 3 years ago

Hmm I'm going to re-open this because the --prepend-file leaves something to be desired. Ideally you would be able to keep common connector and cable definitions in a separate folder and point to it. But doing this breaks paths to image files referenced by the definitions.

The PR #189 already implements relative image paths differently, but your use case is still an issue and I describe the root cause in https://github.com/formatc1702/WireViz/pull/189#discussion_r593457127.

formatc1702 commented 3 years ago

There has been some progress in #189 regarding this issue, @gmkado please have a look and feel free to post feedback.

liambeguin commented 2 years ago

What about having something like this:

templates:
  includes:
    - includes/DB9.yaml
    - includes/molex.yaml
    - include/inline_headers.yaml

This would make it explicit that this specific harness file depends on a list of "libraries"

Thanks for your work on this tool! It's really great at making harness drawings more efficient!

drid commented 4 months ago

What about having something like this:

templates:
  includes:
    - includes/DB9.yaml
    - includes/molex.yaml
    - include/inline_headers.yaml

This would make it explicit that this specific harness file depends on a list of "libraries"

Thanks for your work on this tool! It's really great at making harness drawings more efficient!

I agree with that format, this would also make preview with vscode plugin possible.

kvid commented 4 months ago

The YAML parser library we currently use seems to need the full YAML contents as one long string before beginning the parsing. That's why it's a bit hard to use YAML for specifying more YAML files to include. I don't know if the suggested #223 might help?

martinrieder commented 4 months ago

@kvid take a look at https://github.com/adobe/himl and https://github.com/zerwes/hiyapyco

The hierarchical loading of YAML files could happen before the parsing in any other library, even the current one.

I still find the concept of using include statements quite promising. Since YAML uses # to start comments, you could theorerically employ any C code preprocessor with:

#include "templates.yml"

If it wasn't for the Windows compatibility, the beginning of a YAML file could even carry a shebang statement like:

#!/usr/bin/wireviz --prepend-file xyz.yml

This could turn your YAML file into an executable script!

Both would also propose a solution to #19. This also relates to the CLI change suggested in #238.

tobiasfalk commented 4 months ago

@martinrieder what is the Problem with windows, is this something purely becaus of the YAML library or what?

tobiasfalk commented 4 months ago

One way could be to do it like/with a preprocessor, that goes through the file befor it is treated like a YAML file.

martinrieder commented 4 months ago

@tobiasfalk see here: https://realpython.com/python-shebang/

The Windows CMD processor does not recognize this statement, but there are workarounds. This method would also rely on STDIN/STDOUT, and which is WIP on #320.

Having the possibility to store CLI arguments in the YAML file would be a great way to generate reproducible output. We should place this header before an eventual --- block start, so it could be processed before any YAML code.

I could come up with a proposal that incorporates #307 and #348.

tobiasfalk commented 4 months ago

@martinrieder Then why not have the input file be a .wz(WireViz) file, this way one can add the association for Windows and the first line for Unix/Linux systems. Then WiereViz takes this file let's a Preprocessor run over it, wich add all the includes and so, and exports a pure YAML file(.wz.yalm) wich then runs through the current code.

Similar to how a C compiler does it.

martinrieder commented 4 months ago

@tobiasfalk @kvid I suggested Jinja in #377 for the HTML output. It could be applied to the input YAML as well!

See the topic on template inheritance from the docs. And here for some examples specific to YAML.

Just a side note: have a look at https://github.com/trailofbits/graphtage, which can be used to diff and merge various files like yaml.

tobiasfalk commented 4 months ago

@martinrieder yes would also work, personally I would prefer C/C++ style, since I am familiar with it

martinrieder commented 3 months ago

@kvid wrote:

The YAML parser library we currently use seems to need the full YAML contents as one long string before beginning the parsing. That's why it's a bit hard to use YAML for specifying more YAML files to include. I don't know if the suggested #223 might help?

@gmkado @tobiasfalk What do you think about the YAML syntax that LamaAni/bole suggests?

settings:
    inherit: False # If true, allow inherit parent folders.
    inherit_siblings: True # If true allow inherit configuration files in the same source directory.
    allow_imports: True # If false dose not allow imports.
    use_deep_merge: True # Merge configurations via deep merge. If false, Only root keys are merged (and overwritten)
    concatenate_lists: True # When merging, append the merged list to the current one.
imports:
    - "**/*.config.yaml" # Recursively import all .config.yaml
    - path: my-config.yaml
      required: False # this item is not required.
      recursive: null # Applies if glob. Import recursively.
environments:
    [env name]:{ config overrides (any)}

It comes really close to what was suggested earlier by @drid and @liambeguin in https://github.com/wireviz/WireViz/issues/220#issuecomment-2112601829

liambeguin commented 3 months ago

It's been a while since I posted that comment, and looking at it again, I think going with Jinja2 would be great! It provides a lot more than just includes and is also something pretty standard people are used to.

I implemented a proof-of-concept here: #382

martinrieder commented 3 months ago

Nice. Thanks for taking the initiative, @liambeguin! I will give it a try ASAP... I would strongly recommend using the # symbol instead of (or additionally) curly brackets. If Jinja statements look like a comment, you could still parse it as an ordinary YAML file. This would allow #348 to work without Jinja as well.

tobiasfalk commented 3 months ago

Yes looks good. As mentioned earlier, could we change the file ending in the same move? Would not only the things mentioned above but programs like VScode could also detect dem as a wz file and have the right autocompletion and so on.

liambeguin commented 3 months ago

I would strongly recommend using the # symbol instead of (or additionally) curly brackets.

I'm not sure I understand what you mean by that. do you have an example?

Jinja is a preprocessor, so if it sees it's keywords, it will interpret them otherwise the file is left untouched and can be parsed just like before.

This would allow #348 to work without Jinja as well.

Schema validation is a great idea! have you considered using pydantic for this?

martinrieder commented 3 months ago

@tobiasfalk please stick to .yaml or .yml as a file extenstion, so that text editors will properly recognize it. You can surely configure them to use any other extension, but it seems inconvenient. We might eventually upload a YAML schema to the Schema Store, which could indicate a file name pattern. Common practice here is to use something like .wireviz.yaml as a file extension.

martinrieder commented 3 months ago

I would strongly recommend using the # symbol instead of (or additionally) curly brackets.

@liambeguin wrote: I'm not sure I understand what you mean by that. do you have an example?

https://jinja.palletsprojects.com/en/3.1.x/api/#jinja2.Environment

@liambeguin wrote:

Jinja is a preprocessor, so if it sees it's keywords, it will interpret them otherwise the file is left untouched and can be parsed just like before.

Understood, but I am afraid that it would confuse syntax highlighting when editing these templates. The hash symbol will make Jinja statements look like YAML comments.

This would allow #348 to work without Jinja as well.

Schema validation is a great idea! have you considered using pydantic for this?

Thanks for the hint on Pydantic. I should put this on the list, though maybe into #223. Have another look at #348: I have just uploaded the YAML schema for Yamale.

liambeguin commented 3 months ago

@martinrieder oh I see. I mean why not, but not all lines will be starting with a comment char. the content of blocks will not make a lot of sense if you don't consider the jinja code.

If you concern is just for code highlighting, parsers that understand jinja usually know that it's a preprocessor and allow for a 2 level highlighting.

Thanks for the hint on Pydantic. I should put this on the list, though maybe into #223. Have another look at #348: I have just uploaded the YAML schema for Yamale.

I didn't know about yamale, interesting.. I usually just do Ruamel + jsonschema, but more recently pydantic has removed the need for manual schema handling.

martinrieder commented 3 months ago

@liambeguin I was really happy to see your PR with the Jinja integration. It provides some great possibilities to generate dynamic content, however I would consider it an advanced feature.

I've been looking for a simple way to do inclusion of YAML files that would not require learning a new syntax. I found that the most intuitive solution is ingesting directory trees of YAML files. Users would not need to learn about Jinja syntax if only static content is required. This would also not exclude using Jinja!

Please see the following implementation in Go that could be employed as an external tool. Divvy Yaml - dy parses a directory tree according to the following rules:

If a Python implementation would be required, then YAML Tree might provide an alternative. There is also Directory-Mapper, which provides a lot more functionality.

Please have a look at those tools and tell me what you think. How could it be combined with the Jinja implementation?

liambeguin commented 3 months ago

@martinrieder I like the approach of having a directory structure interpreted as YAML. I think it would be great to load a library of connectors and wires for example, but I'd keep it limited to that (that lib could also be a part of the repo, allowing to reuse and share many basic components). I think I mentioned it earlier, but I'd also recommend having a single way of including files in the yaml syntax. I'd have the yaml tree loader be a part of the code, with the option of passing in additional paths from the command line (to load a custom library for example).

martinrieder commented 3 months ago

@liambeguin wrote:

I think I mentioned it earlier, but I'd also recommend having a single way of including files in the yaml syntax.

Agreed, though implementing the include statement directly in the YAML syntax can create all kinds of difficulties in the parsing process. Just have a look at the following example and you will recognize that there is more complexity to it than simply including a single file.

What do you think about the YAML syntax that LamaAni/bole suggests?

settings:
    inherit: False # If true, allow inherit parent folders.
    inherit_siblings: True # If true allow inherit configuration files in the same source directory.
    allow_imports: True # If false dose not allow imports.
    use_deep_merge: True # Merge configurations via deep merge. If false, Only root keys are merged (and overwritten)
    concatenate_lists: True # When merging, append the merged list to the current one.
imports:
    - "**/*.config.yaml" # Recursively import all .config.yaml
    - path: my-config.yaml
      required: False # this item is not required.
      recursive: null # Applies if glob. Import recursively.
environments:
    [env name]:{ config overrides (any)}

So, including would require a preprocessing step that merges everything into a monolithic YAML document. You can quickly run into conflicts with duplicate definitions or recursion if it is not implemented properly.

I just now found this 4yo post discussing a similar issue in https://github.com/wireviz/WireViz/issues/19#issuecomment-651599804.

Do you have a recommendation on which library to use for this purpose? What do you think about the ones suggested above and in #223?