-
Notifications
You must be signed in to change notification settings - Fork 197
feat: get_cwd and set_cwd
#1014
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
Conversation
I am honestly not sure how I should go about testing these functions independently
I am honestly not sure how I should go about testing these functions independently
If I uncerstand correctly, it looks like you're already doing that by invoking execute_command_line; that seems like the best solution to me, but perhaps others think of something else..
Those are commits from the other branch @sebastian-mutz, the one in this PR: #1014
I don't think I can use execute_command_line here, because that spawns a child process to run the command and using cd would change the working directory of that child process not the parent one... But maybe there is some workaround I am not aware of
Suggestions for testing (summarising from the chat with Jose):
For testing get_cwd:
gfortran: GETCWD
Intel : GETCWD
Flang: GETCWD
For testing set_cwd:
gfortran: CHDIR
Intel: CHDIR
Flang: CHDIR
Alternatively, using execute_command_line should be fine.
There was a problem hiding this 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. Here is a couple of comments:
- the state handler
errshould be optional. So, the internalhandleing features returning the state variable if present, or triggering an error stop if not. This will make behavior consistent to everywhere else. - For testing: I think set/get can be tested in the same test program imho. Here is an example:
- get current directory and save it to "pwd" variable -> must be successful
- create a new local temporary directory -> must be successful
set_cwdto thatget_cwdshould now return the temporary directoryset_cwdback to "pwd"- check successful
I suggest to merge this PR after directory handling is merged into the library already.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here are a few additional comments. I suggest the err output variable to be made optional in all cases, consistent with the library. call err0%handle(err) already includes the option to halt on error, if err is not present.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you @wassup05. As a follow up, may I suggest that the C->Fortran string conversion routine be put, part of a separate PR, into the stdlib_strings module into the to_string interface.
Thank you for this PR, @wassup05. I suggest to wait another couple of days, and in absence of further comments, I think this PR can be merged.
Depends on #1011
get_cwd (cwd, err)- Gets the current working directory (cwd) of the process.set_cwd (path, err)- Sets the current working directory (cwd) of the process.getcwd, chdiron Unix and_getcwd, _chdiron Windowsstate_typeand both the platform specific error code and the user friendly error message are provided if an error occurs