emory-libraries / eulxml

Utilities for using XPath to map XML data to Python objects and Django forms
http://eulxml.readthedocs.org
38 stars 12 forks source link

add support for multiple titles, and let them have displayLabels #10

Closed bcail closed 11 years ago

rlskoeser commented 11 years ago

Hi, and thanks for this contribution. Would you be willing to modify it slightly? Rather than replacing the existing title_info with a list field (which would break any code that uses the current setup), I would suggest adding an additional mapping of a a list field, e.g. something like title_info_list (titleinfo_list?). eulxml.xmlmap is perfectly happy to have multiple mappings to the same xml nodes, so they should interoperate just fine - the old title_info will continue to operate as it did, and give you the first title_info in a list, and the list will give you access to all of them.

bcail commented 11 years ago

OK, I'll submit a new pull request.

rlskoeser commented 11 years ago

Thanks. FYI, rather than closing a pull request and opening a new one, if you use a topic or feature branch for the pull request, you can update your branch and GitHub will auto-update the pull request with the changes. (Documented here: https://help.github.com/articles/using-pull-requests )

bcail commented 11 years ago

Thanks - just reopened. I updated the commit, so let me know if you're good with it the way it is now.

rlskoeser commented 11 years ago

Looks good. Thanks for adapting your update. Display labels will be nice, too.