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.

Sign up for GitHub

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

Jump to bottom

Fix multiple paths: define the platform-specific join separator #840

Merged
merged 10 commits into from
Dec 10, 2023
Merged

Fix multiple paths: define the platform-specific join separator #840

merged 10 commits into from
Dec 10, 2023

Conversation

zjp-CN
Copy link
Contributor

@zjp-CN zjp-CN commented May 13, 2023

fix #576

zjp-CN added 4 commits May 12, 2023 17:01
@zjp-CN
introduce use env_logger
d1699df 
related to denisidoro#576

For the following config

cheats:
  paths:
    - C:\\Users\\Administrator\\AppData\\Roaming\\navi\\cheat
    - C:\\Users\\Administrator\\AppData\\Roaming\\navi\\cheat

navi now gets incorrect paths on Windows, since the seperator `:` for
path join is a valid component.

[2023-05-12T08:58:26Z DEBUG navi::commands::core] Filesystem(
        Some(
            "C:\\\\Users\\\\Administrator\\\\AppData\\\\Roaming\\\\navi\\\\cheat:C:\\\\Users\\\\Administrator\\\\AppData\\\\Roaming\\\\navi\\\\cheat",
        ),
    )
[2023-05-12T08:58:28Z DEBUG navi::filesystem] filesystem::Fetcher = Fetcher {
        path: Some(
            "C:\\\\Users\\\\Administrator\\\\AppData\\\\Roaming\\\\navi\\\\cheat:C:\\\\Users\\\\Administrator\\\\AppData\\\\Roaming\\\\navi\\\\cheat",
        ),
        files: RefCell {
            value: [],
        },
    }
@zjp-CN
rm env_logger; use tracing instead
818d5cd
@zjp-CN
mv navi.log under config dir; debug folders on Windows
2755d51 
2023-05-12T15:51:54.280707Z DEBUG navi::filesystem: read cheat files in `"C"`: []
2023-05-12T15:51:54.281083Z DEBUG navi::filesystem: read cheat files in `"\\Users\\Administrator\\AppData\\Roaming\\navi\\cheats"`: []
@zjp-CN
Fix multiple paths: define the platform-specific join separator
8a2bf7d
@zjp-CN zjp-CN requested a review from denisidoro as a code owner May 13, 2023 03:35
@welcome
Copy link

welcome bot commented May 13, 2023

Thanks for opening this pull request!

@zjp-CN zjp-CN mentioned this pull request May 13, 2023
Closed
@zjp-CN
Copy link
Contributor Author

zjp-CN commented May 13, 2023

But the preview is broken too in debug mode.
20230513220046

The path is not right.
20230513213140

I think it's related to rust-lang/rust#99931 and rust-lang/rust#59117

@denisidoro
Copy link
Owner

@zjp-CN sorry for taking so long to review this. I'm not able to test this on a Windows machine. Is this a change you're comfortable with landing?

@jackandroselv
Copy link

jackandroselv commented Sep 24, 2023 via email

@zjp-CN
Copy link
Contributor Author

zjp-CN commented Sep 25, 2023

Is this a change you're comfortable with landing?

Yes, I use the compiled navi based on this PR on Windows all the time, and it works fine.

This PR mainly fixes the path parsing on Windows (so nothing changes on Linux), and introduces tracing for logging.

@denisidoro denisidoro merged commit b560ba5 into denisidoro:master Dec 10, 2023
@welcome Welcome
Copy link

welcome bot commented Dec 10, 2023

Congrats on merging your first pull request!

@BrewTestBot BrewTestBot mentioned this pull request Dec 10, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@denisidoro denisidoro Awaiting requested review from denisidoro denisidoro is a code owner

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

Successfully merging this pull request may close these issues.

Nothing of cheats available on Windows
3 participants
@zjp-CN @denisidoro @jackandroselv