Improvements to the System library #1735
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
directory-contents returns a <file-locator> even for a symlink that resolves to a directory. Add a `resolve-links?` keyword to resolve links. This uncovered a bug in resolve-locator such that it would always return a locator of the same class that it was passed, regardless of the file system entity it resolved to.
This is fundamentally a file-system operation so it was a mistake to put it in the locators module in the first place. The "file" terminology matches that used in most other file-system exports. I've switched the way arguments are passed to the internal `%resolve-file` function here. The internal function only deals with strings, instead of locators. This avoids some unnecessary conversions when the user passes a string.
* Unix: Do not call expand-pathname from other file-system APIs. Username expansion
should only be done explicitly by user code.
* Unix: Fixed bug: expand-pathname("~") didn't work because "~" is turned into a file
locator and %expand-pathname only handled directory locators. Same for "~user".
* Windows: Rename %expand-pathname to %absolute-path (which is apparently what it was
being used for) and re-implement %expand-pathname as a no-op.
* Add a test.
* Update documentation. Removed the method docs because they added no information over
what the generic-function doc said.
* Also adds a release note for expand-pathname
The macros were written to save a few characters rather than for clarity and searchability (i.e. grep). The "used by" comments could easily get out of date so I removed them. I added type specs to each constant because it seems like it can't hurt.
Also added `follow-links?` keyword argument and a test. `$EINVAL` became unused and started causing a warning. I removed it from *-magic-numbers.dylan for all platforms without testing since it's not exported and all Unixen share the same implementation of link-target, which was the only place it was used. Fixes dylan-lang#1408
housel
reviewed
Jul 9, 2025
| each directory is represented by a :class:`<directory-locator>`. The "." | ||
| and ".." directories are not included in the result. | ||
| Returns a sequence of locators describing the files contained in *directory*. The | ||
| "." and ".." directories are not included in the result. |
There was a problem hiding this comment.
You can use the :file: role for these
| class of locator, :class:`<file-locator>` or :class:`<directory-locator>`, is | ||
| determined based on the file type of the (fully resolved) link target. | ||
|
|
||
| Note that if a symlink points to another file in *directory* then the resulting |
There was a problem hiding this comment.
Let's use "symbolic link" consistently rather than the colloquial "symlink".
| // expand-pathname generic function has a different purpose. See | ||
| // https://github.com/dylan-lang/opendylan/issues/1728#issuecomment-3014101087 for my | ||
| // reasoning for keeping this. | ||
| define function %absolute-path |
There was a problem hiding this comment.
Perhaps this should still be called %expand-pathname, but it should be using GetLongPathName, per my comment on #1728.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Addresses some of the issues in #1728 and #1408