rtfpessoa / diff2html

Pretty diff to html javascript library (diff2html)
https://diff2html.xyz
MIT License
2.8k stars 273 forks source link

Make templates more flexible #480

Closed phikes closed 11 months ago

phikes commented 1 year ago

Olá! Thanks for the project! I am using it with great success.

tl;dr: Hand in the whole file/line and/or other main entities when rendering templates to make them more flexible when overwritten.

Context

I am currently using the project to render a diff viewer which can also render comments per line. To do this I query through the generated HTML to find the right file and line. This would be much easier if I could override the template. E.g. for a generic line:

<tr>
    <td class="{{lineClass}} {{type}}">
      {{{lineNumber}}}
    </td>
    <td class="{{type}}">
        <div class="{{contentClass}}">
        {{#prefix}}
            <span class="d2h-code-line-prefix">{{{prefix}}}</span>
        {{/prefix}}
        {{^prefix}}
            <span class="d2h-code-line-prefix">&nbsp;</span>
        {{/prefix}}
        {{#content}}
            <span class="d2h-code-line-ctn">{{{content}}}</span>
        {{/content}}
        {{^content}}
            <span class="d2h-code-line-ctn"><br></span>
        {{/content}}
        </div>
<!-- The comment can be rendered here and this element easily found via JS -->
        <div class="comment-slot" data-filename="{{file.newName}}" data-new-line-number="{{line.newNumber}}" data-old-line-number="{{line.oldNumber}}" data-line-type="{{line.type}}"></div>
    </td>
</tr>

However only the variables which are currently used in the built-in templates are handed in.

My proposal is to hand in the "main entity"/file a template is dealing with to enable users to write more powerful custom templates. I have this working on a branch and would gladly raise a pull request. I only need it in generic lines for now, so I'd be curious if we should add this to other templates as well.

rtfpessoa commented 1 year ago

👋 Feel free to send a PR for me to take a look. The only problem with changing the parameters of the templates is that it would be a breaking change. But I am fine with doing a release if it makes sense.

phikes commented 1 year ago

Thanks for the answer :-) What existing code would it break? We can keep the existing parameters for backwards compatibility I think.

rtfpessoa commented 1 year ago

Yup, if we just add new parameters it should be fine. I am not aware of a way to do this dynamically to detect what the template expects in an easy way.

Sent from Proton Mail for iOS

On Sun, Mar 5, 2023 at 09:47, Phillip Kessels @.***> wrote:

Thanks for the answer :-) What existing code would it break? We can keep the existing parameters for backwards compatibility I think.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you commented.Message ID: @.***>

phikes commented 1 year ago

We can just hand in more variables than consumed by the template I suppose? Even if we duplicate the information handed into the template then at least it doesn't break. I will write something up in a PR ASAP