InMemoryFileSystemAccess creates weird paths

[sigiesec]

When I call

var fsa = new InMemoryFileSystemAccess
fsa.generateFile("foo", "...")

the resulting path in fsa.textFiles is "DEFAULT_OUTPUTfoo", which looks pretty weird. I think it would be better to insert a separator after the output configuration name, i.e. "DEFAULT_OUTPUT/foo".

In addition, I found that there is a class org.eclipse.xtext.junit4.util.InMemoryURIConverter, but it is unrelated to InMemoryFileSystemAccess. I created one for my tests as follows:

@Accessors(NONE)
class InMemoryURIConverter extends ExtensibleURIConverterImpl
{
    val InMemoryFileSystemAccess fileSystemAccess

    override boolean exists(URI uri, Map<?, ?> options)
    {
        fileSystemAccess.isFile(uri.convert)
    }

    override InputStream createInputStream(URI uri, Map<?, ?> options)
    {
        fileSystemAccess.readBinaryFile(uri.convert)

    }

    def String convert(URI uri)
    {
        val path = Path.fromPortableString(uri.path)
        if (uri.scheme == "memory" && path.segment(0) == "DEFAULT_OUTPUT")
            "/" + path.removeFirstSegments(1).toPortableString
        else
            throw new IllegalArgumentException("Bad URI for InMemoryURIConverter: " + uri)
    }
}

Obviously, this only handles the default output configuration. I think this might be useful for others as well and be included with xtext, but it might need to be generalized to support any output configuration.

[cdietrich]

i am not sure how many clients would break by such a change

[sigiesec]

I wasn't sure if the current behaviour is intentional or if it is a bug.

[cdietrich]

the code was changed to the current one from

protected String getFileName(String fileName, String outputConfigName) {
        return "memory:/" + outputConfigName + "/" + fileName;
    }

in 2014

@szarnekow i assume you dont remember why

[szarnekow]

Looks like https://github.com/eclipse/xtext-core/commit/beb372270c91de64c709a50018e3d0c1b555ba73#diff-d1afcb7d9b8b5e017b3b2cc294eb81dc

introduce getFileName which returned a URI syntax (?) and the following commit restored the broken behavior from before:

https://github.com/eclipse/xtext-core/commit/7d957a18f5f20f5c2b9d6688287fe0b8cfa7122c

Since the filename is merely a string and it would indeed look prettier with a /, but semantically both strings serve more or less the same purpose. I'd be happy to use a prettier internal syntax, but apparently we'd need to adjust a few tests cases. To be on the safe side, I'd also introduce a backwards compat mode that omits the '/'

[cdietrich]

but that would break all clients right that dont explicitely opt-in into the compat mode?

[szarnekow]

This would depend on the default ;)

[cdietrich]

well if the default is the current state then it would be basically the same to create a InMemoryFileSystemAccess2 e.g. as a subclass with changed behaviour

[szarnekow]

I've no preference about this.