birkenfeld / fddf

Fast data dupe finder
Apache License 2.0
114 stars 7 forks source link

new features added: #10

Closed manfredlotz closed 6 years ago

manfredlotz commented 6 years ago

Hi Georg, I added new features

I'm pretty new to Rust. So, probably things can be done better.

Cheers, Manfred

birkenfeld commented 6 years ago

Hi, thanks for the PR! I'll leave a few comments inline.

manfredlotz commented 6 years ago

Do you want me to redo things. Or do you want just to do the needed changes yourself?

birkenfeld commented 6 years ago

I'm fine with both, let me know if you're going to update the PR.

manfredlotz commented 6 years ago

I could do it at the coming weekend. Rust coding speed is slow for me as I'm a very beginner in Rust.

birkenfeld commented 6 years ago

There's no need to hurry :)

manfredlotz commented 6 years ago

As you made changes in the meantime I'm not sure if I should create a totally new pull request when having added my stuff?

birkenfeld commented 6 years ago

You can force-push to the old branch, it should update this PR automatically.

manfredlotz commented 6 years ago

Hm, I made changes but I'm still not sure how to make the changes visible here. FWIW, I did a git push --force

birkenfeld commented 6 years ago

Looks like you pushed to master in your repo, but this PR is for the new-branch branch.

manfredlotz commented 6 years ago

Ok, I switched do new-branch and committed my changes.

I have to admit that I'm not satisfied with what I did, simply because I couldn't figure out how to more elegantly deal with errors when Pattern::new or Reges::new freak out. So I have (very ugly) unreachable expression warning which I didn't want to supress as I hope this can be done in a better way.

birkenfeld commented 6 years ago

Didn't see new changes on the branch, so I went ahead and rebased your master branch, it's now in master. The parsing of the expressions is actually very easy with structopt, as you can see here.

Thanks for the PR!