Show plugin: add option to skip module path #120

Kakadu · 2017-02-09T16:22:05Z

We were kind of bothered that show adds filename as prefix for all constructors. So we added very straightforward way to workaround this. What was the expected way to hack this?

@eucpp

whitequark · 2017-02-09T16:26:10Z

This is implemented through global state and is completely unacceptable. Instead, move the state to the ~type_decl_str function.

Kakadu · 2017-02-09T16:50:15Z

Better?

hcarty · 2017-02-09T18:10:11Z

@Kakadu Could you add a doc update to go with it? This looks like a useful feature.

Kakadu · 2017-02-09T18:12:40Z

@hcarty Are you talking about README.md ?

tekknolagi · 2017-02-09T18:13:58Z

@Kakadu that seems reasonable. As someone who is currently trying to use this API and would appreciate this option, docs anywhere would be helpful.

hcarty · 2017-02-09T18:17:30Z

@Kakadu Yes, to echo @tekknolagi. An update to README.md would be useful.

Kakadu · 2017-02-10T08:45:20Z

@hcarty @tekknolagi ^^

hcarty

A few comments on the documentation. Thank you for updating the readme!

hcarty · 2017-02-10T14:16:46Z

README.md

+module X = struct type t = C [@@deriving show] end
+```
+
+This code will create printers which return the string `A.X.C` where `A` is a filename, `X` is a module path and `C` is a constructor name. To skip all module paths the one needs to derive show with `nofullpath` option (which equals to `false` by default)


"which defaults to false" would be more clear here.

hcarty · 2017-02-10T14:19:23Z

README.md

+...
+# X.show (X.C);;
+- : Ppx_deriving_runtime.string = "C"
+```


The first example shows just the code while this one is in the context of a toplevel session. Maybe they should both be in the toplevel or both be just code?

whitequark

After these last two changes I'll merge the PR.

whitequark · 2017-02-10T14:19:56Z

src_plugins/ppx_deriving_show.cppo.ml

@@ -12,10 +12,15 @@ open Ast_convenience
 let deriver = "show"
 let raise_errorf = Ppx_deriving.raise_errorf

+type options_t = { nofullpath : bool }


Just type options.

whitequark · 2017-02-10T14:20:33Z

src_plugins/ppx_deriving_show.cppo.ml

  options |> List.iter (fun (name, expr) ->
    match name with
-    | _ -> raise_errorf ~loc:expr.pexp_loc "%s does not support option %s" deriver name)
+    | "nofullpath" -> nofullpath := Ppx_deriving.Arg.(get_expr ~deriver bool) expr


I'd prefer to avoid negation here. So, the option would be called with_path and default to true.

tekknolagi · 2017-02-10T19:03:01Z

README.md

+module X = struct type t = C [@@deriving show] end
+```
+
+This code will create printers which return the string `A.X.C` where `A` is a filename, `X` is a module path and `C` is a constructor name. To skip all module paths the one needs to derive show with `nofullpath` option (which equals to `false` by default)


What about using the API manually, as in a hand-written mapper?

Yeah, this change can be useful but I'm not psychologically ready to invest more time in it today.

@Kakadu will you before this PR is in?

I don't think so

Kakadu · 2017-02-14T14:48:15Z

@whitequark Can you review again?

4.1.5 is a maintenance release to include support for OCaml 4.05 and OCaml 4.06 in the ppx_deriving 4.1 branch, which does not contain the driverization work that has been causing issues for some users. * Add support for OCaml 4.05.0 and 4.06.0 (ocaml-ppx/ppx_deriving#154, ocaml-ppx/ppx_deriving#155, ocaml-ppx/ppx_deriving#156, ocaml-ppx/ppx_deriving#159). * Consider { with_path = false } when printing record fields (ocaml-ppx/ppx_deriving#157). * Fix comparison order of fields in records (ocaml-ppx/ppx_deriving#136). * Silence an `unused rec flag` warning in generated code (ocaml-ppx/ppx_deriving#137). * Monomorphize comparison function for builtin types (ocaml-ppx/ppx_deriving#115) * Raise an error when `type nonrec` is encountered (ocaml-ppx/ppx_deriving#116). * Display an error message when dynamic package loading fails. * Add a `with_path` option to `@@deriving` to skip the module path in generated code (ocaml-ppx/ppx_deriving#120).

@polyprinter

CHANGES: 5.0 (26/10/2020) ---------------- * Migrate to ppxlib ocaml-ppx/ppx_deriving#206, ocaml-ppx/ppx_deriving#210 (Anton Kochkov, Gabriel Scherer, Thierry Martinez) 4.5 --- * Add support for OCaml 4.11. - `Ppx_deriving.string_of_{constant,expression}_opt` to destruct `Pconst_string` in a version-independent way ocaml-ppx/ppx_deriving#220, ocaml-ppx/ppx_deriving#222 (Kate Deplaix, Thierry Martinez, review by Gabriel Scherer) * Stronger type equalities in `Ppx_deriving_runtime` (for instance, `Ppx_deriving_runtime.result` and `Result.result` are now compatible with all OCaml versions) ocaml-ppx/ppx_deriving#223, ocaml-ppx/ppx_deriving#225 (Thierry Martinez, review by Gabriel Scherer) * `Ppx_deriving_runtime.Option` compatibility module ocaml-ppx/ppx_deriving#222 (Thierry Martinez, review by Gabriel Scherer) 4.4.1 ----- * Add support for OCaml 4.10 ocaml-ppx/ppx_deriving#211 (Kate Deplaix, review by Gabriel Scherer) 4.4 --- * Restore support for OCaml 4.02.3 ocaml-ppx/ppx_deriving#188 (ELLIOTTCABLE) * workaround Location.input_filename being empty when using reason-language-server ocaml-ppx/ppx_deriving#196 (Ryan Artecona) * Add support for OCaml 4.08.0 ocaml-ppx/ppx_deriving#193, ocaml-ppx/ppx_deriving#197, ocaml-ppx/ppx_deriving#200 (Gabriel Scherer) 4.3 --- * use Format through Ppx_deriving_runtime to avoid deprecation warning for users of JaneStreet Base (Stephen Bastians and Gabriel Scherer, review by whitequark) * silence a ambiguous-field warning (41) in generated code ocaml-ppx/ppx_deriving#163 (Étienne Millon, review by Gabriel Scherer) * use dune ocaml-ppx/ppx_deriving#170 (Rudi Grinberg, Jérémie Dimino) * silence an unused-value warning for show ocaml-ppx/ppx_deriving#179 (Nathan Rebours) 4.2.1 ----- * Add support for OCaml 4.06.0 ocaml-ppx/ppx_deriving#154, ocaml-ppx/ppx_deriving#155, ocaml-ppx/ppx_deriving#156, ocaml-ppx/ppx_deriving#159 (Gabriel Scherer, Fabian, Leonid Rozenberg) * Consider { with_path = false } when printing record fields ocaml-ppx/ppx_deriving#157 (François Pottier) 4.2 --- * Add support for OCaml 4.05.0. * Use the `ocaml-migrate-parsetree` library to support multiple versions of OCaml. * Fix comparison order of fields in records (ocaml-ppx/ppx_deriving#136). * Silence an `unused rec flag` warning in generated code (ocaml-ppx/ppx_deriving#137). * Monomorphize comparison function for builtin types (ocaml-ppx/ppx_deriving#115) * Raise an error when `type nonrec` is encountered (ocaml-ppx/ppx_deriving#116). * Display an error message when dynamic package loading fails. * Add a `with_path` option to `@@deriving` to skip the module path in generated code (ocaml-ppx/ppx_deriving#120). The homepage for the project has now moved to: <https://github.com/ocaml-ppx/ppx_deriving> 4.1 --- * Fix type error with inheritied polymorphic variant type in [@@deriving map]. * Fix incorrect handling of multi-argument constructors in [@@deriving show]. * Add API hooks for ppx_type_conv. 4.0 --- * Show, eq, ord, map, iter, fold: add support for `Result.result`. * Ppx_deriving.Arg: use Result.result instead of polymorphic variants. * Ppx_deriving.sanitize: parameterize over an opened module. * Add support for `[@@deriving]` in module type declarations. * Add support for loading findlib packages instead of just files in ppx_deriving_main. * Treat types explicitly qualified with Pervasives also as builtin. * Compatibility with statically linked ppx drivers. 3.1 --- * Show, eq, ord: hygienically invoke functions from referenced modules (such as X.pp for X.t when deriving show) to coexist with modules shadowing ones from standard library. * Iter, map, fold: hygienically invoke List and Array functions. 3.0 --- * Implement hygiene: Ppx_deriving.{create_quoter,quote,sanitize,with_quoter}. * Show, eq, ord: add support for `lazy_t`. * Add support for `[@nobuiltin]` attribute. * Add Ppx_deriving.hash_variant. * Remove allow_std_type_shadowing option. * Remove Ppx_deriving.extract_typename_of_type_group. 2.1 --- * Fix breakage occurring with 4.02.2 w.r.t record labels * Fix prefixed attribute names (`[@deriving.foo.attr]` and `[@foo.attr]`). * Add allow_std_type_shadowing option for eq and show. 2.0 --- * Add support for open types. 1.1 --- * New plugin: create. * Show, eq, ord: handle `_`. * Show, eq, ord, map, iter, fold: handle inheriting from a parametric polymorphic variant type. * Make `Ppx_deriving.poly_{fun,arrow}_of_type_decl` construct functions in correct order. This also fixes all derivers with types with more than one parameter. * Add `Ppx_deriving.fold_{left,right}_type_decl`. 1.0 --- * Make deriver names lowercase. * Remove Findlib+dynlink integration. All derivers must now be explicitly required. * Allow shortening [%derive.x:] to [%x:] when deriver x exists. * Make `Ppx_deriving.core_type` field optional to allow ignoring unsupported [%x:] shorthands. * Add support for [@@deriving foo { optional = true }] that does not error out if foo is missing, useful for optional dependencies. * Rename ~name and ~prefix of `Ppx_deriving.attr` and `Ppx_deriving.Arg.payload` to `~deriver`. * Renamed `Ppx_deriving.Arg.payload` to `get_attr`. * Add `Ppx_deriving.Arg.get_expr` and `get_flag`. 0.3 --- * Show, Eq, Ord, Iter, Fold: handle ref. * Show: handle functions. * Show: include break hints in format strings. * Show: pull fprintf into local environment. * Show: add `[@polyprinter]` and `[@opaque]`. * Add `Ppx_deriving.Arg.expr`. 0.2 --- * New plugins: Enum, Iter, Map, Fold. * All plugins: don't concatenate affix if type is named `t`. * Add `[%derive.Foo:]` shorthand. * Show, Eq, Ord: add support for list, array, option. * Show: include full module path in output, including for types with manifest. * A lot of changes in `Ppx_deriving interface`. 0.1 --- * Initial release.

Kakadu force-pushed the master branch from ca6c50d to a04fe2e Compare February 9, 2017 16:49

Kakadu force-pushed the master branch 2 times, most recently from 87c0115 to 2549f76 Compare February 10, 2017 08:44

hcarty reviewed Feb 10, 2017

View reviewed changes

whitequark requested changes Feb 10, 2017

View reviewed changes

tekknolagi reviewed Feb 10, 2017

View reviewed changes

Kakadu added 3 commits February 11, 2017 23:06

Show plugin: add option to skip module path

2f7af00

Test for showing without full paths

9f21651

Doc updated: with_path option for show deriver.

1c65259

Kakadu force-pushed the master branch from 2549f76 to 1c65259 Compare February 11, 2017 20:16

whitequark approved these changes Feb 16, 2017

View reviewed changes

whitequark merged commit 14df45f into ocaml-ppx:master Feb 16, 2017

avsm mentioned this pull request Aug 22, 2017

+ppx_deriving.4.2 ocaml/opam-repository#10136

Merged

gasche mentioned this pull request Nov 25, 2017

ppx_deriving 4.1.5 ocaml/opam-repository#10853

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Show plugin: add option to skip module path #120

Show plugin: add option to skip module path #120

Kakadu commented Feb 9, 2017

whitequark commented Feb 9, 2017

Kakadu commented Feb 9, 2017

hcarty commented Feb 9, 2017

Kakadu commented Feb 9, 2017

tekknolagi commented Feb 9, 2017

hcarty commented Feb 9, 2017

Kakadu commented Feb 10, 2017

hcarty left a comment

hcarty Feb 10, 2017

hcarty Feb 10, 2017

whitequark left a comment

whitequark Feb 10, 2017

whitequark Feb 10, 2017

tekknolagi Feb 10, 2017

Kakadu Feb 11, 2017

tekknolagi Feb 14, 2017

Kakadu Feb 14, 2017

Kakadu commented Feb 14, 2017

Show plugin: add option to skip module path #120

Show plugin: add option to skip module path #120

Conversation

Kakadu commented Feb 9, 2017

whitequark commented Feb 9, 2017

Kakadu commented Feb 9, 2017

hcarty commented Feb 9, 2017

Kakadu commented Feb 9, 2017

tekknolagi commented Feb 9, 2017

hcarty commented Feb 9, 2017

Kakadu commented Feb 10, 2017

hcarty left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

whitequark left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Kakadu commented Feb 14, 2017