burtcorp / jmespath-java

A Java implementation of JMESPath
BSD 3-Clause "New" or "Revised" License
169 stars 39 forks source link

string manipulating functions #36

Closed gypapp closed 6 years ago

gypapp commented 6 years ago

The company I am working for consumes tons of markup (XML/HTML) and JSON documents a day. We extract content from markups by XPath and by JmesPath from JSON documents, respectively. Moreover, minor adjustment of the content (ie. manipulation) is sometimes desired during the extraction, and it is specific to the source so it happens on-the-fly by expression doing the extraction.

JmesPath lacks functions we usually use in our XPath expressions. To fill this gap in JmesPath we added several basic string manipulation functions to our function registry. For interoperability and to reduce the learning curve for our support engineers, these functions follow the behaviour of their XPath counterparts as much as we think it is worth doing so.

We hope that these functions could be useful for wider audience and thus this PR makes sense.

iconara commented 6 years ago

Hi, sorry for not responding sooner to this PR. I'm currently on vacation.

Thank you for the contribution, it's much appreciated that you're investing your time in this library.

I will have to think a bit about the suggestion of adding non-standard functions to the core library. I'm not sure it's appropriate, maybe there should to be a separate library with contributed functions?

The fixes and spelling corrections I would be happy to merge though.

gypapp commented 6 years ago

@iconara just take your time. No worries if you conclude that it should go to a separate library. Could it be a separate submodule or even a separate repository?

iconara commented 6 years ago

I think a separate repository would probably be the best – I'm also thinking that accepting custom functions would mean extra maintenance (and you've noticed how long it takes me to respond to things as it is now). With a separate repository I could give people like you commit permissions.

linehrr commented 6 years ago

in our company we also implemented things like RegularExpressionFunction and other String manipulation functions and even some time functions like now() and also some arithmetic functions like subtract(), multiple() etc. it would be indeed helpful if we can share and maintain a common UDF repo, so we don't end up writing the same code again and again.

gypapp commented 6 years ago

@linehrr nice to hear that. @iconara what shall be the next step then? I am happy to continue the contribution just let me know how I could help the most.

iconara commented 6 years ago

@linehrr that makes me very happy to hear, I would love to give you commit access to a contrib repo too!

@gypapp next step is me creating a repository and some basics (build scripts and whatever), and then you can open a PR against that repo with the functions. I'll also merge the fixes from this PR into this repo, you don't have to do anything more with this PR if you don't want to. I'll try to get this done this week, but as I mentioned I'm on vacation (which kind of means I have time for this, but also kind of not, it all depends on the weather, which looks to be rainy this week, so looking good for this kind of thing).

iconara commented 6 years ago

I've created https://github.com/burtcorp/jmespath-java-contrib so please re-open the non-core parts of the PR there. I'll try to get the other parts merged here.

iconara commented 6 years ago

The fixes from this PR were merged in https://github.com/burtcorp/jmespath-java/commit/2d48c87e918c05cf0a4728c78402e7cc82183645, please check that I got everything, and close this PR if that's the case.

gypapp commented 6 years ago

Thanks a lot. Everything is in that commit.

Can we have a (pre)-release soon with the extended interface?

iconara commented 6 years ago

I've released 0.3.0-SNAPSHOT, you can change the parent of jmespath-java-contrib to that release in your other PR.

linehrr commented 6 years ago

@gypapp hi, @iconara created the contrib repo, do you mind put your string operation code there? if you don't have time, I can do that too, but I want your permission since you are the creator of those functions. I will merge in my UDFs as well after that.

contrib repo: https://github.com/burtcorp/jmespath-java-contrib

gypapp commented 6 years ago

@linehrr sure I am willing to do so. Just a couple of issues emerged in the meantime. BTW before getting access to the contrib I've created another PR with almost the same content: https://github.com/burtcorp/jmespath-java-contrib/pull/1

Now I'll try to push it directly to the contrib repo.

gypapp commented 6 years ago

@linehrr since the master is a protected branch someone should approve that PR to make it happen.