Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

feat: exists to return the type of a path #1026

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

Open
wassup05 wants to merge 21 commits into fortran-lang:master
base: master
Choose a base branch
Loading
from wassup05:exists

Conversation

Copy link
Contributor

@wassup05 wassup05 commented Aug 2, 2025

User facing stuff added are

  • integer function exists (path [, err]) to check if a file exists and return it's type.
  • integer parameters, type_unknown, type_regular_file, type_directory, type_symlink which are the return values of the exists function.

To accomplish this, on unix lstat is used and on windows GetFileAttributesA is used.

  1. If any errors, type_unknown is returned
  2. the code and the string describing the code are provided through FS_ERROR_CODE (using strerror on unix and FormatMessageA on windows).
  3. and to accomplish this platform based error handling c_getstrerr now takes a logical winapi argument to switch between these two functions (strerror and FormatMessageA).

zoziha reacted with rocket emoji
Copy link
Member

@sebastian-mutz sebastian-mutz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this PR, @wassup05!
Functionality seems fine; would strongly recommend commenting the code a little more here and there to make it easier to maintain down the line by people other than those intimately familiar with the system modules.

perazz reacted with thumbs up emoji
Copy link
Member

@perazz perazz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this PR @wassup05, I think the implementation is almost ready. The biggest comment I have is about harmonization with the rest of the library.

We already have is_directory, so I think we should introduce similar functionality to is_link and is_file. Same thing for delete_file and remove_directory , we probably need unlink (or remove_link) and create_link?

case S_IFREG: type = type_regular_file; break;
case S_IFDIR: type = type_directory; break;
case S_IFLNK: type = type_symlink; break;
default: type = type_unknown; break;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we differentiate between a missing and an invalid object? What do other libraries do in this respect?

Copy link
Contributor Author

@wassup05 wassup05 Aug 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it is possible to do that, because the respective syscalls may error out, then we don't truly know the status of the path, whether it is missing or invalid, also windows only returns INVALID_FILE_ATTRIBUTES, not differentiating between some runtime error or the path not existing, that part is done by the GetLastError and I think we could also keep it that way for simplicity.

perazz reacted with thumbs up emoji
Copy link
Member

@perazz perazz Aug 6, 2025
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok so for now, I agree to just leave it unknown and assume we cannot differentiate between an actually missing entity and a malformed path yet.

Copy link
Contributor Author

wassup05 commented Aug 6, 2025
edited
Loading

We already have is_directory, so I think we should introduce similar functionality to is_link and is_file. Same thing for delete_file and remove_directory , we probably need unlink (or remove_link) and create_link?

Regarding this there is one thing I wanted to discuss.. in unix all the types such as S_IFDIR etc are mutually exclusive, meaning only one of the 6 types will be set, but in windows they are not, which means a file CAN have more than one attributes set... This could lead to a potential misunderstanding that just because a path is fs_type_symlink it cannot be fs_type_directory which is in fact not true, this is how directory symlinks work on windows...

So I wonder if we should go ahead with these is_ functions instead of trying to assign a unique type to a path? Also should we include more is_ functions that are not really cross platform? Like Python's os.path.is_junction

Copy link
Member

perazz commented Aug 6, 2025

This is the way that feels most natural to me:

  • regular file -> is_file = .true., is_directory = .false., is_symlink=.false., exists = type_file
  • regular dir -> is_file = .false., is_directory = .true., is_symlink=.false., exists = type_dir
  • link to file -> is_file = .true., is_directory = .false., is_symlink=.true., exists = type_link
  • link to dir -> is_file = .false., is_directory = .true., is_symlink=.true., exists = type_link

But of course I suggest further ideas about this be discussed.

sebastian-mutz reacted with thumbs up emoji

Copy link
Contributor Author

wassup05 commented Aug 6, 2025

That seems reasonable @perazz, I have added is_symlink and is_regular_file.
Let me know if it looks good and I will add the specs and examples

sebastian-mutz reacted with thumbs up emoji

Copy link
Member

@sebastian-mutz sebastian-mutz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @wassup05. The reworked and new docs and addition of is_regular_file and is_symlink look good to me. Let's wait a little to see what others think.

Copy link
Member

perazz commented Aug 12, 2025

LGTM @wassup05 @sebastian-mutz, except I is_file looks more natural to me than is_regular_file, and in line with is_directory (we don't have is_regular_dir there). What do you think?

sebastian-mutz reacted with thumbs up emoji

Copy link
Contributor Author

is_file does feel better and concise, but because of the parameter fs_type_regular_file and to be consistent with that I have named it is_regular_file.. I don't have a particular preference as such

sebastian-mutz reacted with thumbs up emoji

Copy link
Member

Hi @wassup05. Perhaps this consistency matters a little less than having a slightly more intuitive is_file, which users will be more exposed to? That said, I see your logic and honestly have no strong opinion either way.

perazz and wassup05 reacted with thumbs up emoji

Copy link
Contributor Author

Another point to discuss I feel is, should these is_ functions provide optional error handling capabilities using the usual state_type instead of just being logical?

Copy link
Member

Can you highlight some specific advantages you see in using state_type/what error handling capabilities you think would be beneficial to these?

Copy link
Contributor Author

wassup05 commented Aug 15, 2025
edited
Loading

It's the same as many other functions that have been added over time, let's say exists in this current PR itself, it takes an optional, intent(out) argument of type state_type and let's say there is some error like the many errors described here (in the ERRORS section) the err argument properly describes what happened.

The same reasoning can be applied to is_directory, is_file, is_symlink.. instead of just a plain .false as the return value regardless of any ERROR that may have occured, we could properly provide the user with the specific details of what actually went wrong.

sebastian-mutz reacted with thumbs up emoji

Copy link
Member

@perazz perazz left a comment
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM pending the attached comment @wassup05, thank you for this PR.
Regarding the logical functions, I agree that (separation of concerns) they should not return a state flag, but just match the underlying logic such is done already.

Think about condition "value>0"
-> true if value = 1.0, 1234.5, +Inf
-> false if value = 0.0, -123. NaN, -Inf

So it seems consistent to me that the edge cases are already considered by the boolean condition.

wassup05 reacted with thumbs up emoji
This function checks if a specified file system path is a symbolic link to either a file or a directory.
Use [[stdlib_system(module):is_file(function)]] and [[stdlib_system(module):is_directory(function)]] functions
to check further if the link is to a file or a directory respectively.
It is designed to work across multiple platforms. On Windows, paths with both forward `/` and backward `\` slashes are accepted.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we are advertising it, would it be reaonable to add a test that checks ./test_file on all systems, and also .\test_file on Windows

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think I understand what you mean, do you mean a sort of utility test function to make the tests simpler and extensive by checking both the kinds of paths for all the tests?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe it would be useful if the tests tested filename but also ./filename (on all OSes) and .\filename (on Windows only)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't believe that's necessary @perazz, because we haven't written any custom logic for that ourselves, it is how the underlying C functions that do the syscalls behave, so adding tests for such would be like testing the C standard library or the Windows API and not necessarily the test of our own application or library.

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

@perazz perazz perazz approved these changes

@zoziha zoziha zoziha left review comments

@sebastian-mutz sebastian-mutz sebastian-mutz approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /