Have coerceToPathname use Gray stream PATHNAME generic #651
Conversation
| @@ -2143,12 +2143,10 @@ public static Pathname coerceToPathname(LispObject arg) | |||
| return (Pathname) arg; | |||
| if (arg instanceof AbstractString) | |||
| return (Pathname)Pathname.create(((AbstractString)arg).toString()); | |||
| if (arg instanceof FileStream) | |||
There was a problem hiding this comment.
From visually inspecting things, I'm surprised that such elision for all the types works. From what I recall, there's a set of fairly specialized code paths in the URL-PATHNAME and JAR-PATHNAME constructors getPathname() calls that wouldn't be invoked with this patch as far as I understand.
The tests didn't complete 'cuz gitlab.common-lisp.net was wedged for the ANSI-TEST suite. I've fixed that, re-running tests now. I should look into how we handle various URL-PATHNAME/JAR-PATHNAME tests which I think haven't worked since "googlecode.com" went away that I should fix and/or update.
We'll see…
There was a problem hiding this comment.
gitlab.common-lisp.net seems to break a lot. Because Clasp's build system does lots of git cloning and has dependencies on gitlab.common-lisp.net, we've set up a mirror of some of the repos. ansi-test is here https://github.com/clasp-developers/ansi-test if you want to use it. It refreshes every day. There is also mirror of asdf and a few others.
There was a problem hiding this comment.
Added @Override for the various getPathname implementations.
There was a problem hiding this comment.
I've added tests to nontrivial-gray-streams for the other pathname components. They all pass for this PR.
There was a problem hiding this comment.
Testing looks good wrt. ABCL's PATHNAME specializations (JAR-PATHNAME, URL-PATHNAME).
Mebbe I will add NONTRIVIAL-GRAY-STREAMS to the CI's testing when I get a chance.
|
Thanks @easye! |
It appears #650 is not quite sufficient for PATHNAME. The spec states that PATHNAME-TYPE, PATHNAME-HOST, etc, should work after close. This means that
coerceToPathnamewill need to call the generic PATHNAME. To accomplish this I've addedgetPathnameto the Stream class.I've made this a draft because I haven't finished writing tests yet and I haven't verified that TRUENAME works in all cases.