Handle variable reference in include

[thSoft]

The following is valid LilyPond:

foo = "foo"
\include \foo

But Elysium says Couldn't resolve reference to Assignment 'include'.

[nittka]

My plan would be to allow an expression after an include. If we can resolve that expression ourselves ending in a string, that string is used as include, otherwise we create a validation error explaining to the user that we cannot resolve the include.

One challenge will be not to use Xtext's linking while resolving the include, or doing it in a smart way as the the includes are need (be)for(e) doing the linking.

[nittka]

A general point would be to improve the import scheme in general. One decision to be made is whether to use importUri or namespace imports. I think, this issue can be addressed with both approaches.

A second question wrt. tests is performance. Currently, resolving all imported URIs is done for each test over and over again. That is, default imports are processed once for transitively determining the imports and once again for creating obtaining the scope from the resource descriptions - neither imports nor exported objects will change for the default imports. Slightly modifying the LilyPondImportUriGlobalScopeProvider using a static ResourceSet for obtaining the imported resources and a static Map<URI,IResourceDescription> causes the execution time for Integration test to drop from 223 seconds to 37 seconds. I would like to have that kind of performance boost for integration tests because, when working on the grammar, these tests have to be run quite often. The cache already in use does not help much because the key is the resource - a different one everytime even for default imports.

For tests, working with static fields should be OK, but it's hard to guess the impact for the application. There re-resolving default imports is not that big a performance problem at the moment(?). I suggestion introducing a little service that works as currently but has a "caching" implementation for the test project.

[thSoft]

This is also a great enhancement for the tests! I would also avoid introducing static state to the application, I don't think re-resolving has huge performance impact.

[nittka]

Using imported namespaces has one big advantage - it would solve the problem of re-parsing and indexing the default imports. However, with LilyPond's include mechanism, it as a big disadvantage, too - uniqueness of the namespace.

Includes can be absolute or relative to the given file. That means the same file name can be used over and over again. With namespaces, the location of the file does not matter, though. So you'd have to find a way to provide a unique namespace and and translate all versions of an include (relative/absolute) to the correct one.

[nittka]

At least in case of typical .ily files this issue cannot be resolved 100% see https://github.com/thSoft/elysium/blob/e305e2b05fb9d368257c1617fc0a85009ee71eb2/org.elysium.test/resources/real-world/Mendelssohn-O42/part.ily

Here an include is parametrized but the definition is not to be found in an included file. (This is true not only for includes but other references as well).

To resolve this (more general) problem, I am thinking about a validation preference page where you can manually adapt the error level for several error types (including no validation)

• unresolvable include variable in .ily (default no validation)
• unresolvable variable in .ily (default no validation)
• unresolvable include in .ly (default error)
• unresolvable variable in .ly (default error)
• version missing (default warning; also depending on whether .ly or .ily?)
• list of variable names for which no linking error should be raised (default discant...; i.e. definitions that are known not to be parsed with the current Elysium version as they are scheme definitions or in scheme imports)

At the moment, I would approach issue 86 using URI imports. It is closer the the include semantics, even if namespace imports would allow for some performace improvements (in particular wrt. default imports). One question is whether you want some general variable resolving, a very simple one (\include "file.ly", or \include \variable where variable points to a string) or a simple one (a chain of variables ending in a string). I don't know the usage scheme of includes with variables. Is dereferencing once enough?

Another problem that is opened up by allowing variable references in includes is that collecting included or including files becomes harder (impossible if you want 100% correctness). The variable may be defined (multiple times and differently) within the same file but also within included files.

[thSoft]

Another option is to mark this case (variable reference in include) as a warning or info stating that it is "unsupported" so there may be bogus Xtext errors afterwards. What do you think?

I agree that the import URI mechanism fits much more for LilyPond's behavior, so let's not change it to the namespace-based one.

Making validation levels configurable is reasonable. I think missing version should be ignored by default in .ily files anyway.

[nittka]

In this case, I would start with the validation preference page, warning that includes with variables are not handled by the tooling. Even with includes whose variable is uniquely defined within the same file, we cannot be sure that it is the right one, I guess. An include after the variable definition may override it. Trying to implement this feature seems like opening Pandora's box. Should it be left closed or should be open it a little?

[nittka]

A first naive implementation accepts simple variables (syntactically), does not process them but informs about potential problems. Do you think this is enough?

[thSoft]

Yes, this is a reasonable approach.

[nittka]

Closed due to previous comment.