Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow easy extention of diff2html #35

Closed
jung-kim opened this issue Dec 1, 2015 · 6 comments
Closed

Allow easy extention of diff2html #35

jung-kim opened this issue Dec 1, 2015 · 6 comments

Comments

@jung-kim
Copy link
Contributor

jung-kim commented Dec 1, 2015

So as you know Ungit has been using diff2html and it has been great.

However, for implementing features like FredrikNoren/ungit#585, it would be rather difficult to do that without modifying the diff2html's source code. In fact, recent new feature "git commit on line level" feature had to do code replace like we do here on some of the diff2html's html code.

Is there an easier way to expose and extend diff2html for use cases like this?

@rtfpessoa
Copy link
Owner

@codingtwinky I can see the idea, but I do not think I completely understand what you need.

From what I understand for the expand part of the code, it would be enough if I received either the files source or their path in the system.
The other feature you mentioned I did not quite understand.

If the idea is just to inject new lines in the diff I can see two ways of doing it:

  1. Receive the extra lines when parsing and create the whole HTML with hidden parts that can be showed later
  2. We parse the diff normally, and then when someone clicks to expand the diff we generate lines on demand and some JavaScript can inject those lines in the middle of the diff.

Do you have any other suggestions?

Is this what you need?

@jung-kim
Copy link
Contributor Author

jung-kim commented Dec 4, 2015

That would work for this specific feature, but to keep in more generic and workable, I guess what I had in my mind is an easy way to inject additional codes in various places.

I originally thought about doing something like how templating languages work, but now that I have laid out this thought and think more and more I'm not sure if I like this approach.

We may have to give up and derive our own getPrettyHtml functions using diff json or hijack and override some of the function within ungit.

i.e.

<tbody class="d2h-diff-tbody">
  <tr>
    <td class="d2h-code-linenumber d2h-del"> 
      ... 
    </td>
    <td class="d2h-del">
      <div class="d2h-code-line d2h-del">
        <span class="d2h-code-line-prefix">
          -
        </span>
        <span class="d2h-code-line-ctn"> 
          ... 
        </span>
      </div>
    </td>
  </tr>
</tbody>

Can be transformed to something like this below with lots of hints for me to easily replace and transform html.

<tbody class="d2h-diff-tbody" diff-file-name="a/b.txt">
  <!-- diff2html start-file-diff -->
  <tr line-number="23">
    <!-- diff2html start-line-diff -->
    <td class="d2h-code-linenumber d2h-del"> 
      ... 
    </td>
    <!-- diff2html mid-line-diff -->
    <td class="d2h-del">
      <div class="d2h-code-line d2h-del">
        <!-- diff2html start-line-diff-detail -->
        <span class="d2h-code-line-prefix">
          -
        </span>
        <!-- diff2html mid-line-diff-detail -->
        <span class="d2h-code-line-ctn"> 
          ... 
        </span>
        <!-- diff2html end-line-diff-detail -->
      </div>
    </td>
    <!-- diff2html end-line-diff -->
  </tr>
  <!-- diff2html end-diff -->
</tbody>

@rtfpessoa
Copy link
Owner

@codingtwinky the HTML transformation looks good to me.
If you want to do some changes and add more functionality I am always available to accept any pull request.

@rtfpessoa
Copy link
Owner

@codingtwinky, After thinking a little bit more about this, I am considering to have some kind of add-ons that would act on the content to do extra behavior:

  • Lazy loading content:

    Instead of invoking the generate html for all the content, parse the diff and invoke file by file as the page goes down.

  • Expanding files:

    Inject some JS triggers to generate more context lines

It would be a good idea to have some kind of pipe and filter, after json generation, before html generation, ..

Still it is kind of a big change, do you think this could help your needs?

@jung-kim
Copy link
Contributor Author

jung-kim commented Dec 9, 2015

It would definitely help but I absolutely agree that it is a big change and I'm not sure if I can selfishly ask for such big changes for us only. As I think more and more I may just ask for more modularized and easy to extend object structure that we can override the certain key functions.

I will try several things and will request or send PRs as things come up.

@rtfpessoa
Copy link
Owner

@codingtwinky ok. I do not mind any features prs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants