feat: includes as property

[sschleemilch]

About

The current VSS include mechanism that is based on comments was always bothering me.
That's why this is an alternative backwards compatible way of defining includes.

type: branch nodes can define a list of files to include as children.
The syntax of the file and all include dir searching logic stays the same.

However, I added support for defining includes without the extension and we are searching for vspec and yaml extensions if not defined. This makes the includes less cluttering.

Also, the includes property will be stripped after extracting the include definitions.
Therefore this has no impact on later stages like exporters, models and so on.

Both definitions can be mixed without problems.

Example

A.vspec

A:
  type: branch
  description: d
  includes:
    - B

B.vspec

B:
  type: branch
  description: d
  includes:
    - C

C.vspec

C:
  type: branch
  description: d

Result:

A
└── B
    └── C

[adobekan]

Happiness is reason enough. Looks ok for me.

[erikbosch]

I have no objections to the idea, but we should better have a companion PR extending https://github.com/COVESA/vehicle_signal_specification/blob/master/docs-gen/content/rule_set/includes.md

We should also discuss when to use the variant in the standard catalog. Like do we prefer the new variant over the old? If so do we want to only accept the new variant for std catalog PRs. Or possibly also change it for existing usecases.

I actually do not know if there are any corner cases where the old syntax must be used as the old does not suffice. As I understand from the change it is possible also for the new syntax to have relative search paths like a/b or ../a/b.

[sschleemilch]

I have no objections to the idea, but we should better have a companion PR extending https://github.com/COVESA/vehicle_signal_specification/blob/master/docs-gen/content/rule_set/includes.md

We should also discuss when to use the variant in the standard catalog. Like do we prefer the new variant over the old? If so do we want to only accept the new variant for std catalog PRs. Or possibly also change it for existing usecases.

I actually do not know if there are any corner cases where the old syntax must be used as the old does not suffice. As I understand from the change it is possible also for the new syntax to have relative search paths like a/b or ../a/b.

I can create a companion PR.

Personally I would switch the standard catalog to using this mechanism so that such an important behavior is not encoded in comments of the holding filetype (YAML).

Of course there is always the possibility to drop the support of the inline comments at some point and what would also be possible is to offer a converter. The only corner case I can think of that the new syntax does not support is #include Foo.vspec, so leaving the prefix since the includes property always defines the prefix its defined in. However, I don't think it's a good pattern to have includes that way. But if we think we need them we could extend the new includes syntax to support it.

[sschleemilch]

Does anyone think we would need a magic toplevel property to hold includes as well?
Personally I would restructure the includers to include those things in their branch instead.

[erikbosch]

Does anyone think we would need a magic toplevel property to hold includes as well? Personally I would restructure the includers to include those things in their branch instead.

As long as we only support a single top node (Vehicle) I see no reason to have more. And if more top levels nodes would be supported that could anyway be solved by giving multiple *.vspecfiles as input to the tool.

[erikbosch]

MoM:

• Ticket presented
• Schildt: We shall better stick to one of the options in the std catalog. Makes life easier if someone wants to implement their own tooling

[sschleemilch]

We shall better stick to one of the options in the std catalog. Makes life easier if someone wants to implement their own tooling

Should we vote which one to use and decline this one if everyone wants to stick to the old one?