Add support for `Debug` type class

[JordanMartinez]

See #46 for context.

Drop Show and replace it with Debug as defined here: https://github.com/hdgarrood/purescript-debugged

For context:

• https://harry.garrood.me/blog/down-with-show-part-1/
• https://harry.garrood.me/blog/down-with-show-part-2/
• https://harry.garrood.me/blog/down-with-show-part-3/

[hdgarrood]

Just to be clear - I would also love to see this happen, but I personally don't have the time/energy to make it happen myself right now.

[JordanMartinez]

That's fine!

Also, I believe you said previously that supporting Debug would require adding support for it in the compiler, right?

Meaning, the work needed to add this would be:

• merge the purescript-debugged code into this repo, so that commits still point to original author
• add support for it in compiler
• make a new release of Prelude with this in it
• in next breaking change, add a deprecation notice to Show ?
• in next breaking change, deprecate Show

[hdgarrood]

I did think that Debug would require compiler support at one point - in particular, without RowToList, compiler support would have been necessary - but I don't think it is required any more. I think we should try to avoid building knowledge of Debug into the compiler unless we really have to.

[JordanMartinez]

I looked at this briefly.

Here are the steps I followed when migrating purescript-generics-rep into prelude. We'd likely do something similar.

As for porting that library over here, it doesn't look like we can port over the entire thing as is. Rather, we'd have to port half of it to enable instances for the Debug class and leave the other half there where diffing and pretty-printing are handled. I'm not sure how that affects testing the implementations and it risks one part being developed in isolation from the other.

The rest of this comment explains how the port would work.

purescript-debugged currently defines 5 five files. In the below list, those appearing in the later part of the list depend on earlier parts of the list. Each will be covered below:

• Type.purs: the heart of the library
• Class.purs: defines the Debug type class
• Generic.purs: enables genericDebug
• PrettyPrint.purs: self-explanatory
• Eval.purs: used for REPL sessions

The Data.Debug.Type file has 4 sections:

• A strict rose tree definition for Tree. Since prelude defines Array's Functor instance, porting this is straight forward.
• The type and smart constructors for Repr and Label as well as some other functions. Since Debug uses Repr, we need to port this for this to work. There are two issues with doing so:
  • record, makeProps and assoc use Tuple. This can be resolved by changing the Tuple to a Record.
  • relativeError uses the Math library, which is defined outside of this one. As such, eqRelative and labelApproxEq, which implicitly depend on that, cannot be ported. I don't think this is a problem because these seem to only be used in the "diffing" part of this file:
• The Delta and DeltaRepr types and related functions used to diff two Repr values. This definitely cannot be ported because it uses functions from Array like zip and the aforementioned relativeError.
• Some pretty printer code, which definitely cannot be ported due to the usage of Maybe, String.

The Debug.purs file consists of:

• the class itself, which can be ported
• two derived functions that use the pretty printing code from Type.purs, so these cannot be ported
• instances for Prim and Prelude types, which can be ported
• instances for everything else in the ecosystem, which would be ported to their respective libraries

The Generic.purs file can be ported easily.

The PrettyPrinter.purs file and the pretty-printing code from purescript-debugged would likely stay in the library. I'm not sure how that would affect the dependencies for that library as I think most of its dependencies exist so it can add Debug instances for those types.

The Eval.purs file would likewise stay where it is since it can't be ported due to depending on PrettyPrinter.purs.

To summarize, here's what would change if we did this:

• we port the Debug type class here, but not its two derived functions.
• we port the Debug instances to their respective libraries
• we update the dependencies of purescript-debugged and leave everything else there.

If one wants to diff and/or pretty-print things produced by the Debug type class, they have to add a dependency on purescript-debugged to gain that functionality. I think it would make sense to absorb purescript-debugged into the core libraries but I don't think it's necessary.

[JordanMartinez]

There's another issue I missed in my initial review. The Record instance for Debug builds an Array via Array.fromFoldable listOfTuples. Since arrays isn't included in prelude, we don't have a way to push values to the Array. This could be done via FFI, but then other backends have to implement this, too.

[JordanMartinez]

I realized that we use FFI for Array.cons in our Show type class instance for Array, so using the same FFI for the Record's Debug type class should be fine.

I forked the purescript-debugged repo, and reorganized the library. You can see the diff here.

[thomashoneyman]

We talked about this briefly in the Contributors meeting last week, and I'm also a 👍 for adding Debug to the Prelude. I don't have much to say about the code itself, but I am concerned about how this would be rolled out to the community.

My reading of debugged is that it's largely a replacement for Show. Ranging from most to least extreme, it feels like these are the main possibilities for moving forward:

1. Remove Show
   I've heard arguments before that Show is a terrible class that shouldn't have been in the language to begin with, and that we could remove it altogether in favor of Debug. This would involve deprecating Show for a cycle, implementing Debug for everything in core / contrib / etc. that uses Show (or switch to a better function, like Int.toString), and then removing Show altogether. The downside: massive breakage, because Show is used all over the place. Possibly this could be mitigated by adding a little show-compat library that provides Show for projects that don't want to switch, and warns that Debug is better, but at least then users could install show-compat for their own code. (I don't love this.)
2. Remove Show instances only
   We could also be a little less dramatic by maintaining the class, so that user code still works fine, but remove all instances from core / contrib / etc. as part of the Show deprecation. The class remains around in Prelude, but instances aren't provided. The downside: a vestigial class lingering in Prelude that we don't want people to actually use.
3. De-prioritize Show
   If we determine that the massive breakage implied by removing Show isn't met with at least equal benefit, then we could still implement Debug instances for everything in core / etc. and judiciously remove Show instances in places where we deem it especially dangerous, but otherwise leave Show alone as a class and leave most instances in place. We could make an effort to change testing libraries in the ecosystem (like spec, where I'm a maintainer) to use Debug instances instead of Show instances, which should be a huge social pressure help towards converting code to Debug. Some possible downsides: Show is still around and in use, and we now have two type classes overlapping in purpose.
4. Introduce Debug only
   We could introduce Debug and add instances for it to core / contrib / etc., and perhaps still make an effort to switch testing libraries over to it, but leave all existing Show instances in place to avoid breaking anything. The downside: The same as the previous downside, but worse, because Show remains pervasive.

I'm primarily in support of the "De-prioritize Show" option; I really don't want to cause a ton of breakage in order to ditch Show, since I think most users will find it more of an irritation than anything else, and I don't really want to introduce Debug without a plan to reduce the usage of Show, since that adds a new overlapping (in intent) class without actually switching to it.

I'm also in favor of switching purs repl to use Debug by default -- specifically because it makes it more likely that teaching materials use Debug instead of Show -- but I understand being hesitant about adding things to the compiler unless necessary.

[garyb]

In our codebases we have a redefined show in our Prelude that has a warning constraint added, that's another tool we have to throw in the mix of options for proceeding here.

[JordanMartinez]

In our codebases we have a redefined show in our Prelude that has a warning constraint added, that's another tool we have to throw in the mix of options for proceeding here.

Could you clarify this?

[garyb]

I mean in the Prelude we use, there's this:

show ∷ ∀ a. TypeError.Warn (TypeError.Text "Show usage") ⇒ Show a ⇒ a → String
show = Show.show

So if we wanted to de-prioritise show we could perhaps start adding a constraint like that to the instances that now have proper print/debug functions, to ensure that we don't end up with two parallel hierarchies that end up doing the same thing indefinitely. Or we could add it to show directly, etc. Basically another tool in promoting a migration to debug without fully breaking things.

[hdgarrood]

Even getting Debug into core and updating the compiler to use it is not a small project, so I’d suggest not worrying too much about the future of Show until Debug is in core and has real-world use. We will have more information by that point, and also if we decide we do want to deprecate Show, it’s going to be much easier to justify that to the user base if users can already see concrete benefits from Debug.

[thomashoneyman]

Even getting Debug into core and updating the compiler to use it is not a small project, so I’d suggest not worrying too much about the future of Show until Debug is in core and has real-world use. We will have more information by that point, and also if we decide we do want to deprecate Show, it’s going to be much easier to justify that to the user base if users can already see concrete benefits from Debug.

My motivation in asking is that if, for example, we just want them to live side-by-side without intending to phase out the usage of Show, then I'm not sure I'm in favor of taking on the not-a-small-project of moving Debug into core. And in the same way, if the plan were to remove Show altogether, then I'm also not sure I'm on board (due to the breakage).

[hdgarrood]

Right, I see. I wouldn’t want them to live side by side indefinitely either, so I think it’s just a question of how gentle we are about deprecating and removing Show. I do think that Show is enough of a footgun in production code that deprecating it justifies the breakage, if we can manage the deprecation process well. For example, we could be very gentle about it by marking Show as deprecated and leaving it there for two or three breaking releases before actually removing it.

I’d also argue that the cost of having Show in prelude is fairly minimal; the code has barely changed in years. Maybe once a big chunk of the ecosystem has moved away from Show, instead of removing the class entirely, we could move it out to a separate library?

I agree that option 3 probably makes the most sense at first, but I also think we should remove it completely if, say, the migration goes well and in a few major releases’ time we think we can remove it without creating too much hassle.

[JordanMartinez]

I agree that option 3 probably makes the most sense at first, but I also think we should remove it completely if, say, the migration goes well and in a few major releases’ time we think we can remove it without creating too much hassle.

This is currently where my thinking is at, too.

I'm thinking a rollout could be something like:

1. Add Debug to prelude, but don't drop Show or touch its instances. Update education materials to emphasize Debug rather than Show.
2. Get the compiler to support Debug
3. In a future breaking change when we feel it's appropriate (i.e. it doesn't have to be v0.15.0), add a deprecation notice to Show. If everyone is in favor of removing it at that point because Debug was so successful, the notice simply states that it will be removed. If not, it'll say that Show is being moved outside of prelude into a purescript-show library (or whatever we call it).
4. In a future breaking change when we feel it's appropriate, remove the purescript-show library completely.

I think this rollout would be long and gentle enough to reduce breakage issues.