Making progress on relative imports

[Bob Nystrom]

I made a bunch of progress on relative and package imports a while back, but got stuck. I really need to push through and get this done so I'm hoping some feedback from you all will help.

If you haven't seen it yet, take a look at the original proposal here:

https://github.com/munificent/wren/blob/9f79dfcdac466c1198cecedd12fc857d6e62bc36/doc/rfc/0001-smarter-imports.md

Since then, I've been persuaded to not use special syntax for logical imports. Instead, a leading "./" or "../" in the import string indicates a relative import. Otherwise, the import is a logical one:

import "./relative/path"

import "logical/path"

This follows how Node works and people on the mailing list seemed to like it. I've warmed up to it too. I have it more or less working on my machine. The sticking point is this:

Internally, the VM maintains a string->module map to keep track of previously-loaded modules. This ensures a given module is only loaded once when you have shared imports. For that to work, these string keys need to uniquely identify the module.

We also need to ensure the strings for relative and logical modules don't collide with each other. It's probably a bad idea for you to have a relative file named "foo/bar.wren" and a logical one in "wren_modules/foo/bar.wren", but there's nothing preventing that and the VM shouldn't get confused if you do it.

Right now, I retain the "./" at the beginning of import string to distinguish these two cases. It works.

The weird part is that this key isn't just an implementation detail of the VM. It is visible to the user in two places:

1. Stack traces in runtime errors.

So if you have:

// main.wren

import "./dir/other"

// dir/other.wren

Fiber.abort("oops")

And you run:

$ wren main.wren

You get:

oops

[./dir/other line 1] in (script)

[./main line 1] in (script)

Note the "./" in there. Note how it shows up even on "main", which you ran using "wren main.wren" not "wren ./main.wren". The CLI assumes the path you give it is relative and implicitly pre-pends "./" to it.

This feels a little hokey and looks kind of ugly to me. It's probably not the end of the world.

Another option would be to use some other kind of separator like ":" between the package name and subpath for logical imports. This means logical imports would have a ":" and relative ones wouldn't. Something like:

[some_package:path/in/package line 234]

[relative/path/thing line 1] in (script)

[main line 1] in (script)

Whatever separator character we pick would need to implicitly forbidden from appearing in relative path names. Colon is probably reasonable. Space might be too.

If we do this, then we need to figure out how to handle the built-in "random" and "meta" modules. Those are logical names that right now are just those simple strings. I'd probably change them to "wren:random" and "wren:meta".

Thoughts?

2. The module strings passed to the embedder API when resolving foreign classes and methods.

This is, I think, the really weird one. When the host app is resolving a foreign class or method, it needs to know which module the class or method lives in. A single app might have two foreign classes named "Blah" but in different modules.

It uses the same canonicalized module map key string for this too. In practice, what this means is that if you use relative imports to modules containing foreign stuff, the module strings you're given start with "./". If you later decide to use logical imports to reach that module, you'll need to change your C code because then you won't get the "./" anymore.

That seems weird, but I don't have a sense of how much of a problem that is. If you are writing your own embedder, you have total control over how import strings are resolved, so you could choose to not use relative imports at all.

Does this feel weird to you? Should we just try it out and see how it goes?

I know this has been dragging on forever. Part of the reason for that is that I got stuck in analysis paralysis mode around the above stuff and I'm trying to break out of that. Getting something working and landed will be good because it will make it easier to start reusing Wren code across projects.

Let me know what you think!

– bob

[Michel Hermier]

How about transforming relative path to absolute since users will use that to watch/fix code?

[Sven Bergström]

Regarding the uniqueness of the key, I can't recall off hand if the details work this way and I'm in a hurry but:

folderA/some/script.wren
folderA/some/importscript.wren //uses ./script.wren

folderB/other/script.wren
folderB/other/importscript.wren //uses ./script.wren

Are both paths here the same key ("./script.wren") and would this prevent the second one reaching the correct code?

On Sun, Jul 15, 2018 at 2:17 PM Michel Hermier <michel....@gmail.com> wrote:

How about transforming relative path to absolute since users will use that to watch/fix code?

--
You received this message because you are subscribed to the Google Groups "Wren" group.
To unsubscribe from this group and stop receiving emails from it, send an email to wren-lang+...@googlegroups.com.
To post to this group, send email to wren...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/wren-lang/CAAZ5spDWDPuzAibYhbcX4n_MLa%3Di0dA5J6HQAmoB28ceBch4dg%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

[Michel Hermier]

It complicates understanding, since it might be confused with logical path

To view this discussion on the web visit https://groups.google.com/d/msgid/wren-lang/CAOHyZoZ34aBc1YRJT5wh8VyExv1cUE5yFxdiT2FzYO4UWjTJZQ%40mail.gmail.com.

[Bob Nystrom]

On Sun, Jul 15, 2018 at 9:47 AM, Michel Hermier <michel....@gmail.com> wrote:

How about transforming relative path to absolute since users will use that to watch/fix code?

That's what Dart does and I've always found it annoying. You end up with stack traces like:

oops

[/Users/myusername/where/i/keep/my/code/my_app/dir/other line 1] in (script)

[/Users/myusername/where/i/keep/my/code/my_app/main line 1] in (script)

Also, that would really break using that path as the module string in the embedding API because you certainly don't want to look for an explicit hardcoded absolute path there.

On Sun, Jul 15, 2018 at 9:54 AM, Sven Bergström <sv...@underscorediscovery.com> wrote:

Regarding the uniqueness of the key, I can't recall off hand if the details work this way and I'm in a hurry but:

folderA/some/script.wren
folderA/some/importscript.wren //uses ./script.wren

folderB/other/script.wren
folderB/other/importscript.wren //uses ./script.wren

Are both paths here the same key ("./script.wren") and would this prevent the second one reaching the correct code?

No, this works out fine. I didn't spell it out in my email here, but the VM has a "resolution" process where it lets the embedder canonicalize the import string first. During that canonicalization process, the CLI concatenates the import string with the import path of the containing module. So using my in-progress branch, say you have:

// main.wren

import "./folderA/some/importscript.wren

import "./folderB/some/importscript.wren

// folderA/some/importscript.wren

import "./script"

// folderA/some/script.wren

System.print("A's script")

// folderB/other/importscript.wren

import "./script"

// folderB/other/script.wren

System.print("B's script")

If you run main.wren, then the resolved module strings are:

./main.wren

./folderA/some/importscript.wren

./folderA/some/script.wren

./folderB/other/importscript.wren

./folderB/other/script.wren

And there's no collision. So in terms of getting the VM and CLI actually handling and loading relative and logical imports, I've got it all working as far as I can tell. It's mostly just an issue now of whether surfacing those resolved import strings is OK or if I should come up with a different policy for the CLI to use to distinguish module paths inside "packages" versus ones that aren't.

Cheers!

– bob

[Michel Hermier]

Le dim. 15 juil. 2018 à 23:12, Bob Nystrom <munifi...@gmail.com> a écrit :

On Sun, Jul 15, 2018 at 9:47 AM, Michel Hermier <michel....@gmail.com> wrote:

How about transforming relative path to absolute since users will use that to watch/fix code?

That's what Dart does and I've always found it annoying. You end up with stack traces like:

oops

[/Users/myusername/where/i/keep/my/code/my_app/dir/other line 1] in (script)

[/Users/myusername/where/i/keep/my/code/my_app/main line 1] in (script)

It is not an annoyance: it is meant to be correctly identified without ambiguity, copy pastable, or parsable by the UI of the frontend.

Not going global might bring the following questions:

1) where is the base path coming from?

2) how can I access it?

3) how can I modify it?

Which can lead to more complicated problems I think.

Cheers!

– bob

--
You received this message because you are subscribed to the Google Groups "Wren" group.
To unsubscribe from this group and stop receiving emails from it, send an email to wren-lang+...@googlegroups.com.
To post to this group, send email to wren...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/wren-lang/CAMu6JH-KoXmD-Wp%3D%2BuGh%2B5iqN1FaXa%2BB4ACqrjtRv41aShmCog%40mail.gmail.com.

[Sven Bergström]

Regarding stack traces.
I threw an abort in a random place and copy pasted what I see.
As mentioned before, I use .wren explicit extensions on relative files, and use "package: some/path" as a logical one. 

I still like this approach. I chose it based a little on dart but with an enforced space for consistency and clarity.

I also chose it because colon is invalid in many contexts where a path may originate from, like urls, files/folders, networks etc.

It's trivial to identify the difference too, with an index of type check.
Note - In my case they're always relative to the project root, until relative resolve lands.
If I import with the ./ it shows up there, as expected.

I like this because it's clear where the code is from (my code? someone else's code?).

> [Error] eh?

[app/app.wren line 16] in color=(_)

[game.wren line 17] in init ready()

[game.wren line 92] in 

[luxe: project line 644] in launch_game()

[luxe: project line 518] in entry()

Some important notes:
IDEs and path resolution.

In sublime or vscode for example, I have it so if I double click on the error (["game.wren line 17"] line) it will jump to that file, opening it if necessary.

If that file doesn't exist (like `luxe: project` is not a valid file) this becomes a usability annoyance, you can't jump to errors in packages.

This can be solved by a `resolve to real path or null type function` which we also have to use in the debugger, but IDEs can't call that.

Unfortunately a lot of IDEs don't allow you to be more specific (they are a simple regex filter on the output window).

In other words, if the stacktrace itself doesn't include a path that the IDE can find, it will fail to let you jump.
There are probably ways to trick it (by say including the full path way off to the right so it doesn't muddy the view? idk) and use the regex still,

but it does pose a real pain so far. Especially when making packages, since the path is not visible.

IDEs and shared plugin/consistency

Since the embedder can decide how the error output looks, the error matching regex is specific to the embedder.
This means that if an error output is different from the default wren / cli approach, a "wren support for vscode" won't catch the errors either.

Having those errors is fundamental to integration in IDEs in general, since they drive a lot of good UI wins.

At least in most editors the simple regex can be applied to the build command which will probably be unique, but just another thing to note.

Regarding the foreign mappings:
I definitely don't want to rebuild on renaming obviously, but I have to think about that a bit more. 

--
You received this message because you are subscribed to the Google Groups "Wren" group.
To unsubscribe from this group and stop receiving emails from it, send an email to wren-lang+...@googlegroups.com.
To post to this group, send email to wren...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/wren-lang/CAMu6JH-KoXmD-Wp%3D%2BuGh%2B5iqN1FaXa%2BB4ACqrjtRv41aShmCog%40mail.gmail.com.

[Brian Slesinsky]

I agree that ":" in filenames in VM output would be a bit of a pain. But full absolute paths are also annoying since you have to be careful about redacting them when copying them into a public bug report.

On Sunday, July 15, 2018 at 2:36:20 PM UTC-7, Sven Bergström wrote:

Unfortunately a lot of IDEs don't allow you to be more specific (they are a simple regex filter on the output window).

The user, IDE, or some combination of both will need to be able to resolve filenames in both import statements and in VM output.

In the output, I think this could be fixed by making all paths relative to a single directory? It doesn't need to be root. One possibility would be to make them relative to the current working directory at startup, unless the embedder changes this.

If IDE's are okay with using two regexps to get absolute paths, then starting all module files with "wren_modules/" and other files with "./" in the VM output might work well.

[Bob Nystrom]

On Sun, Jul 15, 2018 at 2:35 PM, Michel Hermier <michel....@gmail.com> wrote:

Le dim. 15 juil. 2018 à 23:12, Bob Nystrom <munifi...@gmail.com> a écrit :

On Sun, Jul 15, 2018 at 9:47 AM, Michel Hermier <michel.hermier@gmail.com> wrote:

How about transforming relative path to absolute since users will use that to watch/fix code?

That's what Dart does and I've always found it annoying. You end up with stack traces like:

oops

[/Users/myusername/where/i/keep/my/code/my_app/dir/other line 1] in (script)

[/Users/myusername/where/i/keep/my/code/my_app/main line 1] in (script)

It is not an annoyance: it is meant to be correctly identified without ambiguity, copy pastable, or parsable by the UI of the frontend.

Sure, but the main thing you do with it is read it with your eyes. And for that use case, it's annoying to have to scan past a bunch of path components that aren't really relevant. It's worse when local paths are mixed with logical ones because then you get output like:

oops

[/Users/myusername/where/i/keep/my/code/my_app/dir/other line 1] in (script)

[random line 123] in blah()

[/Users/myusername/where/i/keep/my/code/my_app/main line 1] in (script)

[some/package line 123] in blah()

And the line lengths are wildly different.

Yeah, doing a shorter path is more complex, I agree. I don't think there's a perfect solution. But after something like seven years of staring at full paths in stack traces in Dart as well as using an entire package whose sole purpose is to make stack traces more readable, I'm pretty convinced I don't want to show full absolute paths.

That is, of course, unless you pass an absolute path as your script to the CLI.

Basically, what I have in mind is, for relative imports that aren't in some logical package, the path shown is always derived from the initial path you pass for starting script. I think that's fairly intuitive, though of course I'm up for iterating on that if users turn out to be confused.

On Sun, Jul 15, 2018 at 2:36 PM, Sven Bergström <sv...@underscorediscovery.com> wrote:

Regarding stack traces.
I threw an abort in a random place and copy pasted what I see.
As mentioned before, I use .wren explicit extensions on relative files, and use "package: some/path" as a logical one. 

I still like this approach. I chose it based a little on dart but with an enforced space for consistency and clarity.

I also chose it because colon is invalid in many contexts where a path may originate from, like urls, files/folders, networks etc.

It's trivial to identify the difference too, with an index of type check.
Note - In my case they're always relative to the project root, until relative resolve lands.
If I import with the ./ it shows up there, as expected.

I like this because it's clear where the code is from (my code? someone else's code?).

> [Error] eh?

[app/app.wren line 16] in color=(_)

[game.wren line 17] in init ready()

[game.wren line 92] in 

[luxe: project line 644] in launch_game()

[luxe: project line 518] in entry()

That looks pretty nice. The ".wren" kind of feels like noise to me, but I admit it clarifies that you're looking at a file.

I'm bikeshedding here, but the space after the colon makes it seem less like the path after it is part of the whole identifier. Maybe that's just me.

 

Some important notes:
IDEs and path resolution.

In sublime or vscode for example, I have it so if I double click on the error (["game.wren line 17"] line) it will jump to that file, opening it if necessary.

If that file doesn't exist (like `luxe: project` is not a valid file) this becomes a usability annoyance, you can't jump to errors in packages.

This can be solved by a `resolve to real path or null type function` which we also have to use in the debugger, but IDEs can't call that.

Oh, interesting. My hunch is that this is a solvable problem, but I don't have much personal experience with this. I know the Dart plugin for Atom does some pretty sophisticated stuff, though, so I assume we have some latitude.

IDEs and shared plugin/consistency

Since the embedder can decide how the error output looks, the error matching regex is specific to the embedder.
This means that if an error output is different from the default wren / cli approach, a "wren support for vscode" won't catch the errors either.

Right. Most of what we're talking about here is really just the Wren CLI's policy and other hosts are free to do what they want. But having good conventions that most hosts can follow will make it easier to share tools, which I definitely feel is important.

Maybe the right approach is for me to get what I currently have tested and in a state where it can land on master and then iterate on it from there. It doesn't have to be perfect yet, and getting some real-world feedback would help.

Cheers!

– bob

[Bob Nystrom]

On Sun, Jul 15, 2018 at 5:25 PM, Brian Slesinsky <bsles...@gmail.com> wrote:

I agree that ":" in filenames in VM output would be a bit of a pain. But full absolute paths are also annoying since you have to be careful about redacting them when copying them into a public bug report.

Good point.

 

On Sunday, July 15, 2018 at 2:36:20 PM UTC-7, Sven Bergström wrote:

Unfortunately a lot of IDEs don't allow you to be more specific (they are a simple regex filter on the output window).

The user, IDE, or some combination of both will need to be able to resolve filenames in both import statements and in VM output.

In the output, I think this could be fixed by making all paths relative to a single directory? It doesn't need to be root. One possibility would be to make them relative to the current working directory at startup, unless the embedder changes this.

That's basically what it does now (in my branch). All relative imports end up resolved based on the first path you give the CLI on startup. That path in turn can be relative to the current working directory if you want.

 

If IDE's are okay with using two regexps to get absolute paths, then starting all module files with "wren_modules/" and other files with "./" in the VM output might work well.

I did think about putting "wren_modules" in the resolved string directly, but it's really verbose. I feel like that's an OK name for a directory in the user's program where you want to avoid collisions, but it's pretty ugly in something like a stack trace.

Cheers!

– bob

[Bob Nystrom]

On Sun, Jul 15, 2018 at 2:36 PM, Sven Bergström <sv...@underscorediscovery.com> wrote:

[app/app.wren line 16] in color=(_)

[game.wren line 17] in init ready()

[game.wren line 92] in 

[luxe: project line 644] in launch_game()

[luxe: project line 518] in entry()

I started poking around at this to understand better and I've got a question or two.

So these are the resolved module strings, right? "game.wren" is a relative module. "luxe: project" is a logical module, "project" inside the "luxe" package, right?

What do the import strings for these look like? If I want to import "project" from "luxe", do I do:

import "luxe: project"

import "luxe/project"

?

If I want to have a logical package that only contains a single module, is it:

import "foo: foo"

import "foo/foo"

import "foo"

– bob

[Sven Bergström]

yea, 

package: file
package: folder/file

essentially, the extension is just removed, and the package part is resolved from a folder (like wren_modules or in this case the engine installation for the version that the project has a dependency on. So if you had say, mypackage: file, that would resolve to something like <path>/packages/mypackage/1.0.0/file.wren.
There's a tad more nuance to mine (like how the packages are structured instead of at the root) but that's the idea atm.

import "luxe: math" for Math

import "luxe: input" for Input

in image form with highlighting: https://i.imgur.com/6a7bg1N.png

≥

Secondly, since I'm working in terms of an API, there isn't import "package" right now. 

I can see that resolving to some module.wren/lib.wren/index.wren type thing later.
But, I tend toward explicit/clear over implicit, and the inconsistency between "package: stuff" and "package" I don't like. 

I'd rather keep them all consistent personally. so a single file inside a package is just "package: file".

≥
Since I prefer all paths (in a project) are absolute to the project root, I don't have file-relative imports.
I won't prevent file relative when I merge with master, I'll see how it feels but it's a user directed choice if it just works.
In my case file relative and project relative are distinct too. "./file-relative.wren" and "project-relative.wren" are separate right now.
The relative becomes more important for packages themselves.

To add, my approach is more exploring while I work and settling on a final decision when I get there.
So far, I like how it feels and how it works and have been using it as is the last > year+. 

We're soon working on a wren implementation of the pub solver from newest dart,

which around then I'll actually start looking at it more closely.

--
You received this message because you are subscribed to the Google Groups "Wren" group.
To unsubscribe from this group and stop receiving emails from it, send an email to wren-lang+...@googlegroups.com.
To post to this group, send email to wren...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/wren-lang/CAMu6JH9-P6D5fy4-HJnySOJOKSQGoavDxRtYTeZNMQdkbZbt%2BQ%40mail.gmail.com.

[Michel Hermier]

Le lun. 16 juil. 2018 à 04:48, Sven Bergström <sv...@underscorediscovery.com> a écrit :

yea, 

package: file
package: folder/file

essentially, the extension is just removed, and the package part is resolved from a folder (like wren_modules or in this case the engine installation for the version that the project has a dependency on. So if you had say, mypackage: file, that would resolve to something like <path>/packages/mypackage/1.0.0/file.wren.
There's a tad more nuance to mine (like how the packages are structured instead of at the root) but that's the idea atm.

import "luxe: math" for Math

import "luxe: input" for Input

in image form with highlighting: https://i.imgur.com/6a7bg1N.png

To play the devil here, it's looks a lot like a variation of URL format...

To view this discussion on the web visit https://groups.google.com/d/msgid/wren-lang/CAOHyZoZzGt-rK_4-HMNPTfcM14huOyerrdA2LD4stdYdQ4%3D%2B%3DA%40mail.gmail.com.

[Bob Nystrom]

On Sun, Jul 15, 2018 at 7:48 PM, Sven Bergström <sv...@underscorediscovery.com> wrote:

yea, 

package: file
package: folder/file

essentially, the extension is just removed, and the package part is resolved from a folder (like wren_modules or in this case the engine installation for the version that the project has a dependency on. So if you had say, mypackage: file, that would resolve to something like <path>/packages/mypackage/1.0.0/file.wren.

Yup, that's what I was thinking too.

 

There's a tad more nuance to mine (like how the packages are structured instead of at the root) but that's the idea atm.

import "luxe: math" for Math

import "luxe: input" for Input

in image form with highlighting: https://i.imgur.com/6a7bg1N.png

≥

Secondly, since I'm working in terms of an API, there isn't import "package" right now. 

I can see that resolving to some module.wren/lib.wren/index.wren type thing later.
But, I tend toward explicit/clear over implicit, and the inconsistency between "package: stuff" and "package" I don't like. 

I'd rather keep them all consistent personally. so a single file inside a package is just "package: file".

Yeah, I get the consistency argument. But my experience with Dart is that most packages expose a single module, and it's really annoying to have to repeat the name twice everywhere.

This was the main reason I went with "./" for relative modules — that way a bare identifier can be a module inside the same-named package.

≥
Since I prefer all paths (in a project) are absolute to the project root, I don't have file-relative imports.
I won't prevent file relative when I merge with master, I'll see how it feels but it's a user directed choice if it just works.

When you are within a package/project, I think it's also OK for people to do "absolute" imports relative to the root of the package. Some people do that style in Dart too.

But the main reason I wanted to support relative paths is because you can also have scripts and modules not part of any explicit package.

 

In my case file relative and project relative are distinct too. "./file-relative.wren" and "project-relative.wren" are separate right now.
The relative becomes more important for packages themselves.

To add, my approach is more exploring while I work and settling on a final decision when I get there.

That sounds good to me too. I'll probably merge what I have so far to master just so that it's not lingering in a branch forever and we can start playing with it. But definitely nothing is carved in stone. I'd like to try writing and reusing some Wren code to get a feel for it and we can iterate on it from there.

So far, I like how it feels and how it works and have been using it as is the last > year+. 

We're soon working on a wren implementation of the pub solver from newest dart,

which around then I'll actually start looking at it more closely.

Oh, wow. Exciting! :)

– bob

[Michel Hermier]

I just thought about something from the Linux world. In shell we can use ~ as a shorthand for user path.

Why not use something equivalent and do:

`import "~my_module/my_script.wren"`

I naively used ~ but can be any symbol from the windows path *forbidden* char list.

[Bob Nystrom]

That's interesting, but I think it would just cause more confusion. Users expect that to expand to their home directory and re-interpreting the character to mean something similar but slightly different will probably be weird. In particular, it would mean that the same string does different things when used in an import versus passed to Wren on the command line.

Cheers!

– bob

[Bob Nystrom]

Oh, and a quick update...

I went ahead and landed what I have on master. It's definitely not perfect, but it's been lingering in a branch too long. In order to make progress, I think I need more hands-on experience and feedback both from myself and from users. The most expedient way to get that is to land this stuff on master, so I did. :)

This means that if you sync to the latest Wren, you will likely need to re-write your import strings. If you were previously assuming that all imports were relative to the working directory, you'll need to change those to start with "./" and be relative to the containing file's directory. Let me know if you run into anything weird.

Cheers!

– bob