jekyll / jekyll-watch

:eyes: Rebuild your Jekyll site when a file changes with the `--watch` switch.
MIT License
93 stars 36 forks source link

Fix encoding discrepancy in excluded Windows paths #76

Closed ashmaroli closed 5 years ago

ashmaroli commented 6 years ago

Summary

When a site is in a directory containing characters with accents and similar entities (e.g., testé-çasésûr), the paths were encoded differently which were seen as a separate directory in the site.

Context

https://github.com/jekyll/jekyll-watch/blob/adc010795c4f5536b3e6961954a68ea8d41a5de4/lib/jekyll/watcher.rb#L95-L105

If I were to have a custom destination set to "public", then the absolute_path in the code above gets created with encoding Encoding:UTF-8 while the options["source"] encoding was in Encoding:Windows-1252. This resulted in a "relative_path" (via Pathname) starting with ../ which caused that path to not be included as an ignored_path

#<Pathname:D:/testAc-A§asAcsA»r/public>
#<Pathname:../testAc-A§asAcsA»r/public>

or when stringified:

D:/testAc-A§asAcsA»r/public
../testAc-A§asAcsA»r/public

Steps to reproduce issue (Windows only)

/cc @jekyll/windows

Thanks

To @DirtyF for providing a test repository that led to the discovery of this bug

XhmikosR commented 6 years ago

Shouldn't there be a test case for this?

XhmikosR commented 6 years ago

Oh, wait, there is. Then we should have AppVeyor set up so that this doesn't happen again.

ashmaroli commented 6 years ago

@XhmikosR Do you want to add the appveyor.yml..? AppVeyor has been enabled on this repo..

XhmikosR commented 6 years ago

I could give it a go later, but my guess is you'd handle this faster than me :P

DirtyF commented 5 years ago

@jekyllbot: merge +fix

ashmaroli commented 5 years ago

@DirtyF Before this gets shipped, we need to ensure that this does not lead to any regressions in the encoding issues patched in the current public release..