scottgonzalez / pretty-diff

colorized HTML diffs
MIT License
218 stars 27 forks source link

Suggestion: modularize into core and multiple reporter modules #15

Closed tauren closed 9 years ago

tauren commented 9 years ago

My use case requires different HTML markup than pretty-diff produces, so I will need to fork pretty-diff in order to support my needs. Obviously this is not an optimal solution, but I have some ideas on how the pretty-diff architecture could be changed to make it much more flexible to support my and other's needs.

I would suggest modularizing pretty-diff into a core NPM module (perhaps pretty-diff-core), various reporter NPM modules (pretty-diff-reporter-default and pretty-diff-reporter-split), and a CLI module (pretty-diff). The core module would generate an internal structured data model that describes the output of the diff. It would not generate any HTML, its purpose is to perform diffs and return a JSON data model that represents the diff output.

The purpose of a reporter module is to consume a structured JSON data model produced by core and output HTML. The pretty-diff project would provide multiple "official" reporter modules out of the box for default and split views, but anyone else could write their own reporter module that consumes the core data model.

There are features that all reporters should support, such as word-diff and soft-wrap, so a shared implementation of those features could be available as another NPM module (maybe pretty-diff-reporting-utils). Your reporters would depend on it, and others writing reporters could choose to depend on it or could override with their own implementation.

Lastly, the pretty-diff module would simply provide a CLI that can be installed globally as it currently is today. This module would depend on the core and all of the "official" reporters. Git options would be passed to pretty-diff-core from pretty-diff and the JSON output would be sent to a reporter to generate HTML. The CLI could also open the HTML into a browser as it does today. The CLI would provide a way for users to specify which reporter to use with any reporter available as long as the user has npm installed it. Perhaps usage would look like this:

# These use pretty-diff-reporter-default
pretty-diff
pretty-diff --reporter=default
pretty-diff --reporter=pretty-diff-reporter-default
# These use pretty-diff-reporter-split
pretty-diff --reporter=split
pretty-diff --reporter=pretty-diff-reporter-split
# These uses a properly named non-official custom user reporter
pretty-diff --reporter=supercool
pretty-diff --reporter=pretty-diff-reporter-supercool
# These use improperly named non-official custom user reporters
pretty-diff --reporter=tauren-pretty-diff-reporter-kindacool
pretty-diff --reporter=tauren-kindacool-prettydiff-reporter

I'm envisioning a dependency tree like this:

I considered suggesting a reporter plugin architecture, similar to the way test frameworks support various reporters and plugins (such as Jasmine or Karma). But I think that complicates things more than necessary.

Any thoughts?

scottgonzalez commented 9 years ago

This is way too complicated for what this tool does. I'm not going to split a 100 LOC module into at least three different modules. At best, I'll expose exports from here, but that's it.

tauren commented 9 years ago

Fair enough... Some way to customize the output would be nice.