radiant / radiant-clipped-extension

An asset manager for radiant, derived from paperclipped.
Other
20 stars 18 forks source link

r:asset tag is broken in lib/asset_tags.rb (proposed fix) #53

Open joshie opened 12 years ago

joshie commented 12 years ago

when using <r:asset [name,id]="foo">bar/r:asset, there is breakage

This is fixed for me with the following modification to line 17 in lib/asset_tags.rb

    16:  tag 'asset' do |tag|
    17:    tag.locals.asset = find_asset unless tag.attr.empty?

to

16:  tag 'asset' do |tag|
17:    tag.locals.asset, options = asset_and_options(tag) unless tag.attr.empty?

... At least for me. I don't have a firm enough grasp on the code to know if there are any negatives to this fix.

saturnflyer commented 12 years ago

I'm not sure I understand the problem. Can you elaborate?

joshie commented 12 years ago

Okay If I do

<r:asset title="something">
  <r:asset:url />
</r:asset>

r:asset:url would not return the url for "something".

After my fix, it does.

saturnflyer commented 12 years ago

Thanks for the clarification.

Why not just do

<r:asset title="something">
  <r:url />
</r:asset>

Does that not work? Or am I misunderstanding the use case?

joshie commented 12 years ago

What I am stating is that the <r:asset > block simply does not work unless you put the code fix in there. Anything you reference inside won't be referencing that asset, r:asset:url was just an example. The r:asset tag in it's current state is useless and nonfunctional.

It is so you can do a bunch of things with a single asset without having to keep referencing which asset with every command . (this is handy if, for example, you want to make a snippet that needs a bunch of arguments about the asset to interface to a fancy js that displays it, the snippet can be reused because it just goes inside the block and the snippet code can be generic. You can reference the url, the comment associated with the asset, etc).

saturnflyer commented 12 years ago

Thank you for explaining. Got it. Thanks for reporting this. I'll try to write the supporting specs this weekend unless someone beats me to it.

joshie commented 12 years ago

I do need to note that I may have misunderstood you, and this is the case that you would be telling that that inside the block, that r:url is actually referencing r:asset:url and something like r:caption would do what I expect it to.

However, I am pretty sure (i can't test because my live site actually has my fix in there), that you do get an error message output on your page simply by using the tag.

joshie commented 12 years ago

I just wanted to update that your suggestion does not work. That line of code needs to be updated as per my suggestion (or some other solution) for me to get something usable. Sorry for taking so long to check that.