rtfpessoa/diff2html-cli

Include commit details in diff content

timblaktu opened this issue · 4 comments

@rtfpessoa Great Tool! It's going to enable me to implement submodule-aware changelogs in my Jenkins pipeline projects, with just 3 lines in the Jenkinsfile. :-)

Is there a way to coerce diff2html-cli to include diff details (author, commit msg, etc) into the html output? Seems a pretty basic functionality. If this isn't possible currently, I imagine I could do some variable substitution or code generation to inject substrings from the git diff into your html template to provide some of it, but ideally your tool would just include (perhaps optionally, if you omitted it intentionally for some reason) the author/commitmsg/sha as an inline header before the corresponding diff content.

@timblaktu, the idea of diff2html is to support diff parsing and html generation. Since that kind of information is not available in the diff this would be a completely new type of parsing/sourcing of information.

I think it would be better for you to deal with it from your side. Does it make sense?

Thanks, @rtfpessoa, I feel that what I'm requesting is very much in line with the idea of diff2html.
I think I now understand our differences, and why what I'm requesting isn't obvious to you:

You're expecting your users to generate diffs with git diff, whereas I'm using git log -p --submodule=diff to generate them.

You're expecting your users want to generate a single changeset across 2 commits, whereas I'm generating a sequence of changesets, one for each commit in a range. (Your tool works brilliantly in this case, BTW, except that the context for each diff view is not provided.)

So, in your thinking/design, git diff is used, so there is no commit information included in what diff2html has to parse and convert to html. In my thinking, if you were to simply use 'git log -pto generate the exact same patchfile, you will then have commit information at your disposal to parse and place a simple commit header at the top of eachdiff --git a/file b/file` section of your diff content. This header would show:

short commit sha
author
date
commit message

..which would provide extremely useful context for your users when looking at this diff content. It also would give your users support for including submodules in their diffs.

More detail: As I mentioned in my other issue I'm invoking your tool with -i file, passing it a standard diff patch file (CHANGELOG.txt) as generated from a git log -p call:

git log -p --submodule=diff ${GIT_PREVIOUS_COMMIT}..${GIT_COMMIT} > CHANGELOG.txt
diff2html --style side -i file -F CHANGELOG.html -- CHANGELOG.txt

For each changeset in the git log -p output, you now have a commit header that looks like this:

Author: Tim Black <tim.black@foo.com>
Date:   Fri Dec 6 13:51:40 2019 -0800

    enable pipeline timestamps

which precedes each file diff section which as you know can include one or more "git diff" lines.

diff --git a/foo b/foo
.
.
.
diff --git a/bar b/bar
.
.
.

So, consider a diff on a range of 3 commits, which affect one or both of the 2 files foo and bar. Using git log -p you will have:

Author: Tim Black <tim.black@foo.com>
Date:   Fri Dec 6 13:51:40 2019 -0800

    changed foo and bar

diff --git a/foo b/foo
.
.
.
diff --git a/bar b/bar
.
.
.
commit 1111111
Author: Tim Black <tim.black@foo.com>
Date:   Fri Dec 6 13:51:41 2019 -0800

    changed just foo

diff --git a/foo b/foo
.
.
.
commit 2222222
Author: Tim Black <tim.black@foo.com>
Date:   Fri Dec 6 13:51:42 2019 -0800

    changed just bar

diff --git a/bar b/bar
.
.
.

Your parser would need to know how to recognize and split on the commit header in addition to splitting on the "diff" lines, so you'd collect them into a per-commit list of file diffs, instead of just a flat list of file diffs.

Your presentation of this would then be able to provide the context of each changeset (commit sha, date, author, msg) atop each html diff section.

Make sense? What do you think of this approach?

@timblaktu, thank you for the detailed explanation and examples.
I think I now see what you mean.

In general I do not see a problem in adding support for it.

The main challenge should be parsing the commit information in safe way, but should not be impossible.

If you are interested in contributing something like this we can open an issue in https://github.com/rtfpessoa/diff2html and talk a bit more in the about this.

I am finishing the migration to Typescript and I expect to merge it and release a pre-version before Christmas.

@timblaktu If you plan to comeback to this, let me know.