Dirname and basename

[glennj]

No description provided.

[glennj]

This belongs in essentials, eventually.

[joshgoebel]

Agree, but currently it's problematic for Wren Console to have dependencies on Essentials since theoretically it should be compilable without as well... (and we're still not sure how closely we'll work with the Wren CLI project - where we can't assume Essentials is present).

[joshgoebel]

Could we move this to a CoreExtra class/module (open to naming suggestions) for keeping track of such utilities that can't [yet] be moved to essentials? There is an issue in Wren repo for adding this to core Wren, but no progress on it.

[glennj]

Looking into the build failures...

... Ah, tests can't be conditional. I'll rewrite those.

[glennj]

the resolver Wren kernel (and it's Path class)

Well I see dirname is already implemented. I see 2 choices here:

1. move basename into Path
2. have File.dirname wrap Path.new().dirname

[joshgoebel]

Well, technically with wren-console you could have just written this with Testie. But if we ever hope to upstream it to Wren CLI (which seems possible in this case) then yeah you're stuck with this. It's super annoying.

[joshgoebel]

Well I see dirname is already implemented. I see 2 choices here:

I'm reaching out to the Wren team again to talk about the future... Path is another thing that only exists in Console but not CLI... and if we care about upstreaming patches then just depending on Path would require it also be made part of the public API (it's not currently, it's used internally by cli and resolver modules) and also upstreamed.

[joshgoebel]

You don't need the tests to be so receptive actually, the test scaffold has no idea WHICH line generate output, only the total output plan has to match the output generated... so you can move the expectations up to the initial declarations and then do the testing in a loop.

[joshgoebel]

If you're happy with this I will rebase and merge.

[glennj]

Yes, thumbs up.

[joshgoebel]

if (Platform.isWindows) {
  tests = tests.map {|t|
    t["given"]    = t["given"].replace("/", "\\\\")
    t["expect"] = t["expect"].replace("/", "\\")
    return t
  }.toList
}

Why \\\\? Tests are failing but only on windows...

[glennj]

I forgot about that. The reveal of Path.dirname kinda took the steam out of my sails for this one.

I was fumbling around with different approaches to getting windows tests working, then I realized I shouldn't be using github for my docker testing, then life intervened and my progress stopped.

I was contemplating closing this PR and taking a fresh approach to it. Maybe put it as Draft until the windows bugs are ironed out.

[joshgoebel]

I was contemplating closing this PR and taking a fresh approach to it.

We're still limited by Path being "off limits" though because of official CLI considerations. Hence my jumping back on this and seeing if we can push it to home as-is.

Boom, all tests green. Not sure why you felt the double was needed at all since you're using a single in the line right below?

Unless you can think of something I'm missing I think this is good to merge now.