themekit icon indicating copy to clipboard operation
themekit copied to clipboard

ignore_files not working with directories in paths (on Windows)

Open PhilSole opened this issue 5 years ago • 7 comments

Describe the bug I'm using config.yml like so:

development:
  password: 12345
  theme_id: "112344939"
  store: ashop.myshopify.com
  ignore_files:
    - settings_data.json

That is successfully ignoring settings_data.json but when I have:

development:
  password: 12345
  theme_id: "112344939"
  store: ashop.myshopify.com
  ignore_files:
    - config/settings_data.json

The settings_data.json file is not being ignored and is getting updated on the remote theme.

To Reproduce Steps to reproduce the behavior:

  1. Using the live theme
  2. theme watch --allow-live

Environment (please complete the following information):

  • Windows 10
  • ThemeKit 1.1.1 windows/amd64
  • Editor VS code
  • Ignore:

Additional context Just set up theme kit.

PhilSole avatar Sep 17 '20 09:09 PhilSole

Hi @PhilSole

Thanks for the detailed report. Did you restart theme watch after changing the config?

andyw8 avatar Sep 17 '20 14:09 andyw8

Also, since you're on Windows, could you try writing it as config\settings_data.json or config\\settings_data.json.

andyw8 avatar Sep 17 '20 15:09 andyw8

Thanks @andyw8

I did restart theme watch.

The single backslash works but not the double. Is that backslash the normal syntax in config.yml for Windows?

PhilSole avatar Sep 17 '20 19:09 PhilSole

In Windows, \ is the default path separator. Some programs expect\, but others use / for consistency with other platforms.

I'll keep this issue open until we make a decision on how to handle this.

andyw8 avatar Sep 17 '20 20:09 andyw8

Thanks. Yeah I don't recall the backslash being required in other web dev tools I've used on Windows. Seems like the / is generally the choice for consistency.

PhilSole avatar Sep 17 '20 21:09 PhilSole

I have come up against this before and did a little playing around in this PR https://github.com/Shopify/themekit/pull/658 but never shipped it. Currently yes, I believe that we use OS specific file paths so you might see it work with a backslash instead, this is because I had assumed that most windows developers would be used to using backslashes instead of forward slashes. However it might seem prudent to accept both in configuration and just normalize them for the OS when they get used.

tanema avatar Sep 21 '20 16:09 tanema

As an interim improvement, I've opened https://github.com/Shopify/themekit/pull/803 to update the documentation.

andyw8 avatar Sep 23 '20 02:09 andyw8