borkdude / jet

CLI to transform between JSON, EDN, YAML and Transit using Clojure
Eclipse Public License 1.0
687 stars 42 forks source link

Support built-in case conversions from https://github.com/clj-commons/camel-snake-kebab #93

Closed rlhk closed 3 years ago

rlhk commented 3 years ago

I find it very common to convert keywords to kebab case when working with EDN from JSON.

Understand it's not too hard to do so with 3 extra lines of deps pulling in bb, in order to access the CSK fns. But it'd be very nice to have those fns as options for -k. The lib is tiny after all.

Happy to submit a PR if agreed.

borkdude commented 3 years ago

Is there a reason other than cosmetics that you're not just working with the :foo_bar style keys directly?

I'm not sure yet if adding another dependency is worth it, but I'll leave the issue open for feedback from other users. As you say, it's already possible to do this fairly easily in babashka.

rlhk commented 3 years ago

My main thought is to produce more idiomatic output for the the main language to work with. Say :instance-ids for Clojure or "instanceIds" for JS etc. Not sure if idiomatic means cosmetics :)

Yeah, agree to see feedbacks from other users. Thanks.

borkdude commented 3 years ago

@rlhk I thought this was a pretty interesting article on this subject: https://vvvvalvalval.github.io/posts/clojure-key-namespacing-convention-considered-harmful.html

rlhk commented 3 years ago

@borkdude that's a great relevant share. The article covers a lot of thoughts. Reading the comments section is helpful too. I'm now more convinced that jet as a tool should provide more options. It's a team's decision on which casing convention to use.

borkdude commented 3 years ago

@rlhk ok, let's do it then.

rlhk commented 3 years ago

@borkdude I guess the simplest approach is to have a helper fn to replace eval-string in needed locations:

(defn eval-string-with-csk
  [s]
  (eval-string (str "(require '[camel-snake-kebab.core :as csk]) " s)
    {:namespaces {'camel-snake-kebab.core {'->PascalCase csk/->PascalCase
                                           '->camelCase csk/->camelCase
                                           '->SCREAMING_SNAKE_CASE csk/->SCREAMING_SNAKE_CASE
                                           '->snake_case csk/->snake_case
                                           '->kebab-case csk/->kebab-case
                                           '->Camel_Snake_Case csk/->Camel_Snake_Case
                                           '->HTTP-Header-Case csk/->HTTP-Header-Case}}}))

so this shall work:

(= "{:an-apple 2}\n{:a 3}\n{:a 4}\n"
   (jet "{\"anApple\":2} {\"a\":3} {\"a\":4}" "--from" "json" "--keywordize" "#(-> % csk/->kebab-case keyword)")

If the direction looks ok, I'm wondering re SCI, is there's a way not to list all CSK fns explicitly?

borkdude commented 3 years ago

@rlhk That is going into the right direction. A few things that help speed things up:

1) Create a SCI ctx beforehand. Instead of using (eval-string s opts) we are going to use (eval-string* ctx str) where ctx can be defined as a top level var.

2) Adding the require like that is fine too, perhaps we can also evaluate this before hand in the context already, at compile time.

3) You can add an entire namespace in one go like this:

https://github.com/babashka/babashka/blob/973938134b0678bf45e8c038bac358f2266d6802/src/babashka/impl/rewrite_clj.clj#L37-L67

but often prefer to list them explicitly so I know that we're not adding stuff that nobody is going to use, so you might want to check this.

Also see the SCI readme for more info.