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

Would you welcome a typescript version? #58

Open
rgbkrk opened this issue Feb 14, 2019 · 4 comments
Open

Would you welcome a typescript version? #58

rgbkrk opened this issue Feb 14, 2019 · 4 comments

Comments

@rgbkrk
Copy link

rgbkrk commented Feb 14, 2019

Right now I'm working with a vendorized copy of diff-match-patch as part of CoCalc's code base. I'm considering converting diff-match-patch to typescript as the rest of cocalc's repository is switching over to typescript.

However, I view the source of truth for the implementation to be here in this repository (just like the npm package for diff-match-patch from @JackuB et. al. defers to here). Instead of modifying it "internally", I'd like to contribute upstream here.

I would assume we'd still make the compile target match what's documented in the wiki:

it works in Netscape 4, Internet Explorer 5.5, and all browsers released since the year 2000

As part of this, I'd keep it closely aligned with @types/diff-match-patch.

@NeilFraser
Copy link
Contributor

Hi Kyle,

Yes, we would be interested in a TypeScript version. It would solve a lot of problems with trying to shoehorn the JS version into TS applications while keeping various type checkers happy.

I worked on a TS version a few months ago, and made considerable progress. However, other things got in the way, and I had to put it to one side. It also became apparent to me that I'm the wrong person to make design decisions in TS since I have no expertise in this language. Here's what I have so far, maybe it would be a good starting point:
https://neil.fraser.name/temp/typescript.zip

@dmsnell
Copy link

dmsnell commented Nov 10, 2019

@rgbkrk I'm not a maintainer here so this is only the opinion of someone who also uses the library. The task of rewriting in TypeScript seems like it would be a big enough change that it could be a massive PR hard to review and fully test.

Did you try seeing how hard it would be to start by compiling the existing JS with the TypeScript compiler? Did it have an impact on the output size? Could it obviate the role that Closure Compiler plays?

It seems like if we could make a case for introducing TypeScript into the build step for JavaScript then we could rewrite functions one at a time and slowly lean more heavily on the type safety.

I'd at least be curious to learn from this kind of transition.

@rgbkrk
Copy link
Author

rgbkrk commented Nov 14, 2019

Check out #74 for the current PR.

@ramin-zaghi
Copy link

ramin-zaghi commented Sep 15, 2022

I took the JS and converted to a properly typed TS version for one of our own projects.
It uses TS classes for the three main classes as well.
Most errors were related to possible null values and I addressed them by checks such as "diff?diff.length:0;".

Took a few days but works well.

The test I ran was a simple VSCode extension which can compare two texts.
Here is a simple example based on the original demo:

import * as dmp_module from './diff-match-patch/diff_match_patch_uncompressed';

function test_compare() {
    var dmp: dmp_module.diff_match_patch = new dmp_module.diff_match_patch();

    var patch_text = '';
    var text1 = "hi there, how are you";
    var text2 = "hi there, changed, how are you";
    var diff = dmp.diff_main(text1, text2, true);

    if (diff.length > 2) {
        dmp.diff_cleanupSemanticLossless(diff);
    }

    var patch_list = dmp.patch_make(text1, text2, diff);
    patch_text = dmp.patch_toText(patch_list);
    console.log(patch_text);
}

Here is the PR #127

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

No branches or pull requests

4 participants