New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CL:PROBE-FILE signals an error when passed a directory on CLISP. #8

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
2 participants
@cl-user
Copy link

cl-user commented Aug 2, 2014

Please refer to:

http://clisp.org/impnotes.html#probe-file

Their rationale for this is documented here:

http://clisp.org/impnotes.html#dir-is-not-file

This commit replaces calls to CL:PROBE-FILE with CL-FAD:FILE-EXISTS-P
and CL-FAD:DIRECTORY-EXISTS-P to make clhs work on CLISP.

CL:PROBE-FILE signals an error when passed a directory on CLISP.
Please refer to:

http://clisp.org/impnotes.html#probe-file

Their rationale for this is documented here:

http://clisp.org/impnotes.html#dir-is-not-file

This commit replaces calls to CL:PROBE-FILE with CL-FAD:FILE-EXISTS-P
and CL-FAD:DIRECTORY-EXISTS-P to make clhs work on CLISP.
@Hexstream

This comment has been minimized.

Copy link
Owner

Hexstream commented Aug 3, 2014

Thanks for the report!

I think CLISP is being rather capricious and unpragmatic here.

In your proposed patch, I see that you're importing cl-fad as part of your solution. I'm not keen to import a third-party library (notwithstanding its popularity) for such a trivial fix, especially to paper over a gratuitous implementation-dependent idiosyncrasy.

I think I'll just use a reader conditional to use EXT:PROBE-PATHNAME on CLISP, for the time being. If the probe-file and possible related issues prove to be recurring annoyances in other projects of mine, I might eventually consider adopting cl-fad, but I like to be quite careful about importing third-party libraries, in general. This is about controlling complexity and having the best possible understanding of my systems.

I expect to release a new version of the clhs wrapper within a few days.

@cl-user

This comment has been minimized.

Copy link

cl-user commented Aug 4, 2014

That's fair enough. I will proceed with closing this pull request.

@cl-user cl-user closed this Aug 4, 2014

@Hexstream

This comment has been minimized.

Copy link
Owner

Hexstream commented Aug 5, 2014

Version 0.6.2 of the CLHS wrapper should work correctly on CLISP.

It will be in the next Quicklisp release but you can get it from here:
http://www.hexstreamsoft.com/libraries/clhs/clhs_latest.tar.gz

@cl-user

This comment has been minimized.

Copy link

cl-user commented Aug 6, 2014

I have just tested this, it works fine now. Thanks for taking care of quirky CLISP!

@Hexstream

This comment has been minimized.

Copy link
Owner

Hexstream commented Aug 6, 2014

Awesome! Thanks for the confirmation!

On Wed, Aug 6, 2014 at 10:39 AM, cl-user notifications@github.com wrote:

I have just tested this, it works fine now. Thanks for taking care of
quirky CLISP!


Reply to this email directly or view it on GitHub
#8 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment