google / diff-match-patch

Diff Match Patch is a high-performance library in multiple languages that manipulates plain text.
Apache License 2.0
7.46k stars 1.11k forks source link

Would you welcome a typescript version? #58

Open rgbkrk opened 5 years ago

rgbkrk commented 5 years ago

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 commented 5 years ago

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 commented 4 years ago

@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 commented 4 years ago

Check out https://github.com/google/diff-match-patch/pull/74 for the current PR.

ramin-zaghi commented 2 years ago

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