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

Figure out relationship to diffs-rs #1

Closed
mitsuhiko opened this issue Jan 24, 2021 · 5 comments
Closed

Figure out relationship to diffs-rs #1

mitsuhiko opened this issue Jan 24, 2021 · 5 comments

Comments

@mitsuhiko
Copy link
Owner

Currently this does not use diffs-rs but it tries to stay largely compatible to it. The reason it's a fork rather than reusing the code comes down to the following reasons at the moment:

  • I want to replace the difference.rs crate with this crate and my desire is not to introduce additional dependencies
  • The source code of diffs-rs is hard to read and documentation is very, very light
  • The diffs-rs source code is tracked as part of pijul which is not a very common tool and it's rather cumbersome to find the source code and issue tracker of it if one runs into issues.

The outcome right now is not super great because it should be possible in theory to create a version of this crate which is reduced to exactly what diffs-rs does.

@mitsuhiko
Copy link
Owner Author

mitsuhiko commented Feb 1, 2021

From the conversation with @P-E-Meunier on a Twitter DM:

We're working on character encodings at the moment in Pijul, which is closely related to diffs, and would probably benefit you. Note that I don't mind if you fork the project.

I believe my main point is that in addition to this, I would be happy to stop maintaining the diffs crate, and switch Pijul to use similar, and contribute to performance improvements, if it were hosted in Pijul, because a fast diff is crucial for me.

So current status is that for pijul to use this it would need to be hosted in pijul. So one option is to separate out the diff part again into a crate and move it off github.

@mitsuhiko
Copy link
Owner Author

I tried using diffs directly for a test now but I'm currently running into incompatible bounds on the patience diff.

@P-E-Meunier
Copy link
Collaborator

You mean the D type parameter requires Sized in the following function?

https://docs.rs/diffs/0.4.1/diffs/patience/fn.diff.html

@mitsuhiko
Copy link
Owner Author

mitsuhiko commented Feb 1, 2021

@P-E-Meunier I have to admit I have troubles reading the trait bounds on that function at the moment. I believe that the current bounds define S::Output: Sized which does not work for the usage in this crate.

The bounds in similar are like this instead:

pub fn diff<Old, New, D>(
    d: &mut D,
    old: &Old,
    old_range: Range<usize>,
    new: &New,
    new_range: Range<usize>,
) -> Result<(), D::Error>
where
    Old: Index<usize> + ?Sized,
    New: Index<usize> + ?Sized,
    Old::Output: Hash + Eq,
    New::Output: PartialEq<Old::Output> + Hash + Eq,
    D: DiffHook,
{

This lets me have a Index<usize> with Output = str.

@mitsuhiko
Copy link
Owner Author

These crates have diverged too much, I'm going to close this issue is no longer being applicable.

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

2 participants