search

cordawyn / spira

Fork of Spira with experimental branches, occasionally merged upstream.
http://blog.datagraph.org/2010/05/spira
The Unlicense
8 stars 2 forks source link

Localization - Pull request #16

Closed abrisse closed 11 years ago

abrisse commented 11 years ago

See the "Support language tags for literals" thread.

cordawyn commented 11 years ago

@abrisse , could you take a look here: https://github.com/cordawyn/spira/blob/activemodelish/lib/spira/persistence.rb#L432 I'm afraid this will delete all other localized values except the "changed" one upon saving the record. Could you write a test for that?

abrisse commented 11 years ago

@cordawyn I don't see why it would delete all the other localized value. That's the has_many :propertyname_native which allows the persistence of the data (property :propertyname is just the accessor), and you already write all the tests for that.

cordawyn commented 11 years ago

Right, there are no problems with persisting. However, I've got concerned with another possible issue: the localized attribute name is absent in @attributes cache, which is a very important element in ActiveModel. Instead it has "*_native" key. There are at least 2 core methods that rely on @attributes: "read_attribute" and "write_attribute", besides "Dirty" submodule is relying on that too.

Could you think on getting the "normal" localized attribute name into @attributes instead of its "_native" alias? While I don't have use cases where having "_native" key can be bad (off the top of my head, anyway), I'm sure that this will negatively impact the compatibility level of Spira with Rails.

abrisse commented 11 years ago

property name :localized => true create a new attribute name_native which is compliant with the full Spira stack. It creates some localized accessors like name and name= that access/modify the real attribute using its accessors directly. So for me everything is fine. Please feel free to modify my commit if something bothers you.

cordawyn commented 11 years ago

As I said before, my goal is to provide a better compatibility with Rails. I see your change as one breaking that compatibility. On the other hand, I realize that you're probably the only person providing use cases to drive the development, now that I don't use Spira ;-) I suggest that we release Spira gem without this pull request, then merge it and release another version. Deal?

abrisse commented 11 years ago

That's totally fine by me! :)

cordawyn commented 11 years ago

@abrisse since I'm going to ask you to merge the HEAD into this pull request again (as it cannot be automatically merged now), could you send it to ruby-rdf instead, to save time?