jgm / pandoc

Universal markup converter
https://pandoc.org
Other
34.07k stars 3.35k forks source link

lens? #3360

Open tonymorris opened 7 years ago

tonymorris commented 7 years ago

I have a need to use the lens machinery on the pandoc definition data types. I am doing technical writing (aviation) against the pandoc syntax tree (Text.Pandoc.Definition). I know there is a pandoc-lens package but that doesn't quite cut it. I have the following options:

  1. write an extra module, but that will have orphan instances.
  2. write an extra module, then wrappers of pandoc data types to write instances against, but this will be a bit clumsy to use.
  3. write the code within the module Text.Pandoc.Definition but that means a dependency on lens.

My preference is for 3. but of course, a dependency on lens may not be desirable.

Any comments?

tonymorris commented 7 years ago

There is also an option of depending on profunctors and contravariant (from memory, might be a bit off), which are much smaller than lens. This would mean a bit of extra hand writing, but that's an insignificant penalty imo.

jgm commented 7 years ago

I don't know much about lenses. I've tried to keep pandoc-types lightweight, and lens seemed big.

But let's see.

pandoc-types depends on aeson.

aeson depends transitively on:

array attoparsec base base-compat binary bytestring bytestring-builder containers deepseq dlist fail ghc-prim hashable nats primitive scientific semigroups tagged template-haskell text time time-locale-compat transformers transformers-compat unordered-containers uuid-types vector

So, adding lens would add these dependencies (besides lens itself):

base-orphans bifunctors comonad contravariant distributive exceptions filepath free generic-deriving kan-extensions parallel prelude-extras profunctors reflection semigroupoids StateVar stm void

That's quite a lot.

There seems to be a tradition of creating packages like pandoc-lens, aeson-lens, etc. Although I assume these all involve orphan instances, it's probably a place where it makes sense to use orphan instances.

What's wrong with pandoc-lens, and have you asked the author if you can make it better?

tarleb commented 7 years ago

My understanding is that lens is not a typeclass but a simple type alias, which can be reproduced easily without requiring the lens package.

type Lens s t a b = forall f. Functor f => (a -> f b) -> s -> f t 

Lenses for pandoc types could be added based on that, without external dependencies.

Most notably, there won't be orphaned instances, whether lenses are defined within pandoc-types or in a separate module. I only skimmed over the pandoc-lens package, but it seemed fine to me. What are the issues with it?

tonymorris commented 7 years ago

The pandoc-lens package uses orphan instances, which I'm not very comfortable with. It also doesn't generalise some of the lens values in a way that I believe would be particularly useful for pandoc. That itself is easy to fix, but the orphan instances would have to remain.

@tarleb You are correct and that is option 4, requiring only a dependency on depending on profunctors and contravariant. I might try experimenting with this option.

jgm commented 7 years ago

I like the sound of that 4th option.

tonymorris commented 7 years ago

I'll give it a crack.

tonymorris commented 7 years ago

I have been able to get pretty far, but I need the dependency on lens, otherwise it becomes a lot of extra work [I don't mind that so much], but also, I cannot implement all the useful things [I do mind]. For example, I cannot implement Wrapped instances.

I have also tidied up some of the core code where it re-implemented lenses, but that was due to a name clash.

I have prototyped this to the point that I have authored a document using the definition types (and their associated lenses, prisms, traversals) and the compiled it out.

jgm commented 7 years ago

Okay, that's disappointing that a full lens import is needed.

It might be better to develop this as a separate package, despite the orphan instances that involves. Not sure.

In any case, can you point to some code so we can see both what the lens definitions look like and an example of how you are using lens?

tonymorris commented 7 years ago

Hey @jgm yeah I do, but it's very prototypical and I am considering changing it up. Stand by for a while.

tonymorris commented 7 years ago

I have implemented the lenses for Text.Pandoc.Definition here https://github.com/tonymorris/pandoc-types/commit/4ec4759d4c9d1843831d01ba7d373863f4f636cb

I have also introduced lens to the pandoc package itself so that I could instance some of the type-classes. There are lots of opportunities to take advantage of lenses in this package.

https://github.com/tonymorris/pandoc/commit/040e6ed2f934d1ed68c2ea02156af6e55150ba5a