metafacture / metafacture-core

Core package of the Metafacture tool suite for metadata processing.
https://metafacture.org
Apache License 2.0
69 stars 34 forks source link

Improve handling of XML attributes and element values. #394

Closed blackwinter closed 2 years ago

blackwinter commented 2 years ago

Make attribute markers configurable:

Make value tag names configurable:

Fixes #379.

Related to #336.

blackwinter commented 2 years ago

I added a commit to reuse some variables

I like the sentiment, thanks. But I see two issues with it:

  1. You included the change with the empty literal names we discussed above. Was that intentional? I actually don't think that's such a good idea. I've pulled it out into a self-contained commit.
  2. For compatibility reasons we can't set the default attribute marker for the handlers identical to the one in the encoder. That's just a minor nuisance, I guess.

as we don't have an official contributing.md yet

We don't? I thought that's what #375 was. How does it become official?

(Not that I mind, though... the rebase ban is one of my major grievances with these guidelines :wink:)

dr0i commented 2 years ago

I added a commit to reuse some variables

I like the sentiment, thanks. But I see two issues with it:

1. You included the change with the empty literal names we discussed above. Was that intentional? I actually don't think that's such a good idea. I've pulled it out into a self-contained commit.

+1 right

2. For compatibility reasons we can't set the default attribute marker for the handlers identical to the one in the encoder. That's just a minor nuisance, I guess.

ack

as we don't have an official contributing.md yet

We don't? I thought that's what #375 was. How does it become official?

ups, right! Well maybe I had slumbering in my mind that you are not so fond of it ...

(Not that I mind, though... the rebase ban is one of my major grievances with these guidelines wink)

but we don't ban it at a whole ... might you want to read again https://github.com/metafacture/metafacture-core/blob/master/CONTRIBUTING.md#force-pushing and discuss your concerns (maybe via email)?