Config setting to add custom composite separator#295
Config setting to add custom composite separator#295fostermh wants to merge 6 commits intockan:masterfrom
Conversation
pull updates from core repo
add composite fields separator config setting
update flatten subfields
ckanext/scheming/helpers.py
Outdated
|
|
||
| @helper | ||
| def scheming_composite_separator(): | ||
| return config.get('scheming.composite.separator','|') |
There was a problem hiding this comment.
Probably it would be better to keep this setting as the default '-' and override if needed.
Could this affect existing schemas? If so we should move this to the scheming yaml / json instead providing a per type setting. What do you think?
There was a problem hiding this comment.
good point about the default separator character. I have made that change. I don't think this will affect existing schemas, now that it is using the correct default separator char. A per field override is an interesting idea though. It would be a bit of work as everywhere the separator is used we would also need to check which field we are handling and check the schema. So far, my experience is that a global setting is meeting all our needs in cioos.
|
I'm testing it with the spatial plugin, and apart few changes (check comments in pull req), it looks working. |
|
Hi so in short it's a very good Idea, I've still a problem on subfields of type multifield. That button was not working due to an issue with the new character on the js.
I'll send a pull req to you so you can integrate in this pull req. |
|
here is it, just a supereasy fix to the js and a line in the documentation: |
|
ok, I merged in your pull request. thanks for adding the README update. It seems the 2.9 tests are failing but I can't seem to get the tests to run locally. not sure what the issue is there. |
|
I'm checking the error and it looks unrelated [#310], although I'm aligning the tests to better reflect the usage of the separator. This patch looks good (tested, worked) and it's very useful when spatial is involved so thank you! |
|
ok great. I think this is good to go then. Not sure who should review next and/or merge? |
2 participants
While working with scheming, spatial, harvester, composite, and repeating extensions I noticed that the default separator of '-' for composite fields caused some problems. This was mostly due to the default field names in the spatial extension containing the same character. I had previously forked the composite extension and added a config setting to set the separator used on composite fields to a custom character.
Now that scheming supports this functionality directly I suggest adding a config setting to allow users to set the separator character to something other than a dash if needed. This pull request addresses this suggestion.
To use a custom separator character, add the
scheming.composite.separatorsetting to your ckan.ini file.Fix #294