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

path::lexically_normal() behaves slightly differently from the standard #33

Closed
rikyoz opened this issue Oct 21, 2019 · 4 comments
Closed
Assignees
Labels
available on master Fix is done on master branch, issue closed on next release bug Something isn't working
Milestone

Comments

@rikyoz
Copy link
Contributor

rikyoz commented Oct 21, 2019

Hi!
First of all, thank you for this library!

Describe the bug
I'm trying to use this library as a fallback when the compiler does not support the final standard version of <filesystem> library.
However, I've found that in some cases the result returned by path::lexically_normal() of ghc is different from the one returned by the C++17 standard version.
In particular, the normal version of path("../") should be "..", while ghc retains the trailing '/' (eventually converted to '\\' on Windows).
On Windows, the same applies also to path("..\\"), which should be normalized to ".." and instead still retains the trailing '\\'.

To Reproduce
To reproduce the behavior, I've created a small program to compare the different results between ghc::filesystem and std::filesystem:

#include <iostream>
#include <filesystem>
#include "ghc/filesystem.hpp"

using std::cout;
using std::endl;

int main() {
    cout << "[std] path(\"../\").lexically_normal() == ";
    cout << std::filesystem::path("../").lexically_normal() << endl;
    
    cout << "[ghc] path(\"../\").lexically_normal() == ";
    cout << ghc::filesystem::path("../").lexically_normal() << endl;

#ifdef _WIN32
    cout << "[std] path(\"..\\\\\").lexically_normal() == ";
    cout << std::filesystem::path("..\\").lexically_normal() << endl;
    
    cout << "[ghc] path(\"..\\\\\").lexically_normal() == ";
    cout << ghc::filesystem::path("..\\").lexically_normal() << endl;
#endif
    return 0;
}

I've tested the program using various compilers and operating systems.

  • On Windows 10 using

    • MSVC 2017
    • MinGW-w64 9.2.0
    • clang 9.0

    the program output is:

    [std] path("../").lexically_normal() == ".."
    [ghc] path("../").lexically_normal() == "..\\"
    [std] path("..\\").lexically_normal() == ".."
    [ghc] path("..\\").lexically_normal() == "..\\"
  • On Ubuntu 18.04 LTS using

    • g++ 8.3.0
    • clang 6.0

    the program output is:

    [std] path("../").lexically_normal() == ".."
    [ghc] path("../").lexically_normal() == "../"

Expected behavior
In all these cases path::lexically_normal() of ghc should return a path("..") in order to conform to the standard.

External References

  • std::filesystem::path from cppreference.com

    A path can be normalized by following this algorithm:
    ...
    7. If the last filename is dot-dot, remove any trailing directory-separator.

  • Relative Paths from Open Standards

    A path in normal form has no redundant current directory (dot) elements, no redundant parent directory (dot-dot) elements, and no redundant directory-separators.

@gulrak gulrak added the bug Something isn't working label Oct 23, 2019
@gulrak gulrak self-assigned this Oct 23, 2019
@gulrak
Copy link
Owner

gulrak commented Oct 23, 2019

Indeed, that is wrong. I'll look into it this evening, after my daytime job.

Thanks for the detailed report!

gulrak added a commit that referenced this issue Oct 23, 2019
gulrak added a commit that referenced this issue Oct 23, 2019
@gulrak gulrak added the available on master Fix is done on master branch, issue closed on next release label Oct 23, 2019
@gulrak
Copy link
Owner

gulrak commented Oct 23, 2019

Okay, fixed it on master. Will also be part of release v1.2.8 (not scheduled yet).

@rikyoz
Copy link
Contributor Author

rikyoz commented Oct 24, 2019

Thanks for the detailed report!

You're welcome!

Okay, fixed it on master. Will also be part of release v1.2.8 (not scheduled yet).

Great, thank you!

@gulrak gulrak added this to the v1.2.8 milestone Nov 6, 2019
@gulrak
Copy link
Owner

gulrak commented Nov 15, 2019

Released with v1.2.8

@gulrak gulrak closed this as completed Nov 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
available on master Fix is done on master branch, issue closed on next release bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants