Skip to content
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

Avoid using "." for the current directory. #959

Open
wants to merge 1 commit into
base: vanilla
Choose a base branch
from

Conversation

isojalka
Copy link
Contributor

All supported platforms support leaving out "./" (or ".") in front of paths to indicate the current directory. However, not all platforms support using "." to indicate the current directory.

Modified the code to avoid constructing paths with "./" or "." when no directory component needs to be prepended to a path.

Copy link
Contributor

@OmniBlade OmniBlade left a comment

Choose a reason for hiding this comment

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

Seems reasonable to me.

All supported platforms support leaving out "./" (or ".\") in front
of paths to indicate the current directory. However, not all
platforms support using "." to indicate the current directory.

Modified the code to avoid constructing paths with "./" or ".\"
when no directory component needs to be prepended to a path.
@isojalka isojalka force-pushed the avoid_dot_current_directory branch from 05ee406 to 30f0345 Compare March 19, 2024 10:32
@th-otto
Copy link
Contributor

th-otto commented Mar 20, 2024

That will result in empty paths to be passed to opendir instead of "." when the search path (ie. as returned from Program_Path()) is also empty. Since that is MorphOS specific, it should be protected by an ifdef.

@isojalka
Copy link
Contributor Author

It is not MorphOS-specific, and getting rid of ifdefs is one of the things I'm trying to do and have been doing a lot. If something needs platform-specific handling, it should go into a platform-specific file.

But this particular problem could also be solved in a different way that'd avoid ending up with a completely empty path. Instead of the current change, PathsClass::Concatenate_Paths() could be modified to handle one or more empty components, and if two empty components are passed to it, it could then return "." instead.

Would such a solution work for you?

@th-otto
Copy link
Contributor

th-otto commented Mar 20, 2024

I i understand it correctly, then the problem is that MorphOS does not handle opendir(".") (see https://github.com/isojalka/Vanilla-Conquer/blob/aa0fd65572e526dbaaff614d787372d1db229767/common/file_morphos.cpp#L97)

But on other os, opendir("") will fail, and that will happen with the above patch.

PathsClass::Concatenate_Paths() could be modified to handle one or more empty components, and if two empty >components are passed to it, it could then return "." instead.
Would such a solution work for you?

I have to check, but that might work. See also #950 where that is already partly implemented.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants