meeech / shopify.tmbundle

A bundle for interacting with Shopify Theme Assets.
http://meeech.github.com/shopify.tmbundle/
136 stars 31 forks source link

"Send Asset" vs "Send Selected Asset" in assets folder #74

Open timdmackey opened 11 years ago

timdmackey commented 11 years ago

Hey meech,

Thanks for the awesome bundle, I can't tell you how much pain this has saved me from.

I have a question about uploading files to Shopify. Why is it that javascript and css assets in the assets folder can't be uploaded using the simple "Send Asset" commend, which sends the frontmost document? As I'm usually switching back and forth between at least a couple files at a time, it can be a pain to have to select the proper files in the sidebar before uploading with "Send Selected Assets." I often find myself wondering why the heck my changes aren't being reflected in my store, until I realize that I have the wrong file selected in the sidebar.

Is this something that can be changed?

timdmackey commented 11 years ago

Ahah! I figured out what the problem was. It turns out that the command was working if you selected it from the menu, but the keyboard shortcut wasn't working because it was set up with a too narrow scope of text.html.liquid. I just removed the scope and this fixed the problem!

On a related note, though, if you try to upload a blank text file (whether its html, js, or whatever), the bundle thinks that you're trying to upload an image and throws the message: *Error: This is an image file. Use Send Selected Assets to Shopify instead. Apparently is_binary() is returning true in upload.php when passed a blank text file. I'm unclear as to why this message even exists though, since as far as I know you could never have an image in the foreground for this command to work on.

timdmackey commented 11 years ago

Okay... removing the scope broke things. I got it working properly by changing the scope for "Send Asset to Shopify" from text.html.liquid to text.html,source.css,source.js. You could possibly add other file types as well to the scope, but these are all that I can see myself needing. Maybe source.json as well?

meeech commented 11 years ago

hi @timdmackey

glad to see you managed to figure it out.

Yes, I don't make those scopes (source.css, source.js) because its overly broad - so its left to the user to add those if they want. thanks for letting me know of is_binary() thinking empty file is binary.

re: the error message- there was a case where it could be reached - i probably misspeak when i refer to images

timdmackey commented 11 years ago

Ah, thanks @meeech. I see what you mean about the scope. As far as textmate bundles go, I'm only really familiar with snippets, I don't really know how scopes and language syntaxes are defined. That being said, how hard would it be to define a source.js.liquid and source.css.liquid scope, and then limit the "Send Asset to Shopify" command to those scopes? This seems like it would be a fair compromise that would help for shopify development, while not getting in the way for non-shopify development.

meeech commented 11 years ago

@timdmackey yeah, that would be a good solution.Its not hard if you know TM language syntax well. Unfortunately, i don't, and it's never been a priority. PRs for this are welcome.