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

Handlebars::register_templates_directory doesn't handle trailing backslash in path correctly on Windows #388

Closed
andrewhickman opened this issue Oct 19, 2020 · 7 comments · Fixed by #389
Labels

Comments

@andrewhickman
Copy link

Minimal repo program:

fn main() {
    let path = std::env::args_os().nth(1).unwrap();
    let mut registry = handlebars::Handlebars::new();
    registry.register_templates_directory(".hbs", path).unwrap();
    println!("{:?}", registry);
}

Run against the following directory structure

test
└ test.hbs

With the path .\test\, outputs:

Handlebars { templates: {"est": Template { ...

Note the first letter of the filename has been lost.

@andrewhickman andrewhickman changed the title Handlebars::register_templates_directory doesn't handle trailing backslash in path correctly Handlebars::register_templates_directory doesn't handle trailing backslash in path correctly on Windows Oct 19, 2020
@sunng87 sunng87 added the bug label Oct 22, 2020
@sunng87
Copy link
Owner

sunng87 commented Oct 22, 2020

Thank you for reporting @andrewhickman. I will look into this later

@sunng87
Copy link
Owner

sunng87 commented Oct 24, 2020

@andrewhickman since I don't have a window machine, could you please help me to verify if #389 fixed this?

@andrewhickman
Copy link
Author

Yep, that's fixed it, thanks!

@nathan-osman
Copy link

I'm still seeing this with 3.5.1 on Windows 10 (20H2) :

let mut tpl = Handlebars::new();
tpl.register_templates_directory(".hbs", "templates/")
    .expect("unable to register templates");

println!("Templates:");
for (name, _) in &*tpl.get_templates() {
    println!(" - {}", name);
}

Assuming I have the following directory tree:

templates
  ├── index.hbs
  └── about.hbs

...the code prints:

Templates:
 - ndex
 - bout

@nathan-osman
Copy link

Peering through the source code, I found this:

let prefix_len = if dir_path.to_string_lossy().ends_with(path::MAIN_SEPARATOR) {
dir_path.to_string_lossy().len()
} else {
dir_path.to_string_lossy().len() + 1
};

Out of curiosity, why is prefix_len incremented by 1 if the native path separator isn't present? The function seems to exhibit the expected behavior if I replace it with the following line:

let prefix_len = dir_path.to_string_lossy().len();

@sunng87 sunng87 reopened this Dec 27, 2020
@sunng87
Copy link
Owner

sunng87 commented Dec 27, 2020

@nathan-osman this is to get the relative path of templates. We don't want the relative path to start with separator, so if given path doesn't end with it, we count it into prefix_len.

The issue seems to be you are using / on Windows, which is not the MAIN_SEPARATOR on Windows. I can fix this by checking either / or \.

@sunng87
Copy link
Owner

sunng87 commented Dec 30, 2020

@nathan-osman 3.5.2 should fix this issue

@sunng87 sunng87 closed this as completed Dec 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants