search

rtfpessoa / diff2html

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

What does matching actually do? #440

Closed akash329d closed 1 year ago

akash329d commented 2 years ago

Hi,

I've been trying to figure out what the matching option actually does, playing around with the demo here - https://diff2html.xyz/demo.html

The output always looks the same regardless of the matching option chosen.

Thanks

rtfpessoa commented 2 years ago

@akash329d I think I might have broken in during the migration to typescript. I need to dig where I might have broken it. I think the difference was how it applied the matching algorithm.

rtfpessoa commented 2 years ago

After a quick look seems like maybe it was something different. Probably I should split the matching from diffstyle.

But seems like it was some improvement to the highlight of the changes in the diff here

Need to check what changed in the diff dependency.

akash329d commented 2 years ago

Have been playing around with it a bit and it seems like the changedWords array is still being created correctly. Might have something to do with how it's being displayed?

As a separate comment real quick, would you be open to adding an option for the library to return JSX rather than an HTML string? (So that it could be used as a react component). I was going to create a PR but figured it would be a pretty large change (and I'm not quite sure how to structure it), so another option could be to fork the library and make a separate diff2html-react version.

rtfpessoa commented 2 years ago

As a separate comment real quick, would you be open to adding an option for the library to return JSX rather than an HTML string? (So that it could be used as a react component). I was going to create a PR but figured it would be a pretty large change (and I'm not quite sure how to structure it), so another option could be to fork the library and make a separate diff2html-react version.

It sounds like something that could be interesting to have. What would be your idea to implement this together with the current HTML output?

akash329d commented 2 years ago

That's the part I wasn't totally sure on. Could make it a flag to return JSX rather than HTML string but then would need to add React as a dependency for the project, and have a some duplicate code. My plan for it was to take all of the Hogan templates and convert them to functions that return JSX. But would likely be better off forking and doing the conversion rather than having it as a flag.

rtfpessoa commented 2 years ago

Could make it a flag to return JSX rather than HTML string but then would need to add React as a dependency for the project, and have a some duplicate code.

Why would you need react as a dependency? Are you returning actual JSX elements? Or Would you be generating a file which has JSX inside?

My plan for it was to take all of the Hogan templates and convert them to functions that return JSX.

If you plan is just to change the templates it should be trivial. You can even try it by overriding the current ones and see if it works for you.

But would likely be better off forking and doing the conversion rather than having it as a flag.

Feel free to do that to.

akash329d commented 2 years ago

I was planning on returning actual JSX elements, so I was thinking that React would be required for that. It might be possible to use JSX without React though. Will try just directly changing the templates and seeing how that goes.

My main goal was to make it into a React component that is easy to drop in to any React project, similar to https://github.com/praneshr/react-diff-viewer

rtfpessoa commented 1 year ago

Closing for inactivity.