tinted-theming / base16-builder-go

A base16 builder written in go, focused on convenience for template maintainers.
MIT License
50 stars 8 forks source link

Fix program to work correctly on Windows #5

Closed martinlindhe closed 7 years ago

martinlindhe commented 7 years ago

Also added a required fix in https://github.com/belak/base16-builder-go/pull/5/commits/88f655d344cfb370502a569a78028b60d9c4bfee, without this change, slugs will be parsed incorrectly resulting in a broken app.

These changes are required for windows support.

The last commit https://github.com/belak/base16-builder-go/pull/5/commits/33752952dc2c1d54335dea0a7d8d803cf73d7e36 is more astethical and might be controversial? It simply prints strings as they are (%q escapes the strings, resulting in output like Directory "templates\\vscode\\themes" does not exist. Creating on Windows)

belak commented 7 years ago

Good catch! I'll try to get this merged in later today when I have a chance to try it out.

martinlindhe commented 7 years ago

Thanks, and yes please try it out before merging! I have not tried this PR on Linux/macOS but i am certain it will work.

I verified it working (on Windows) by building https://github.com/martinlindhe/base16-conemu

Also, I have fixed the same bug for windows compatibility in other golang software before. The rule of thumb is dont use "path" for file paths, instead use "path/filepath"

From https://golang.org/pkg/path/:

The path package should only be used for paths separated by forward slashes, such as the paths in URLs. This package does not deal with Windows paths with drive letters or backslashes; to manipulate operating system paths, use the path/filepath package.

belak commented 7 years ago

Looks good to me. Thanks again!