[Commented On] D12380: contrib: add a partial-merge tool for sorted lists (such as Python imports)

[martinvonz (Martin von Zweigbergk)]

martinvonz added inline comments.

INLINE COMMENTS

> Alphare wrote in Cargo.toml:9
> It would be pretty low effort to still support lower versions of Rust, but going to 1.59 allows the use of edition 2021 and makes sense for a `contrib` file anyway, so I understand the appeal of a newer version.

Agreed, it would be pretty low effort, but yes, I figured it would be fine in `contrib` to not worry about it. Sounds like you're not really objecting either, so I'll leave it as is.

> Alphare wrote in main.rs:147
> Suggestion: with `clap` 3 you can use struct-based arguments. I find the declarative approach clearer and less prone to bugs.

Good idea! (I should do that upgrade in my hobby project too.)

> Alphare wrote in main.rs:166
> Maybe only write the files that have indeed changed?

Heh, that's both clearer and more efficient. Silly me.

> Alphare wrote in main.rs:189
> Why do we want the wrapping behavior here?

It adds an `isize` (offset) to a `usize`. I did that by converting the `isize` to `usize` and then doing a wrapping add. Happy to hear alternatives.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D12380/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D12380

To: martinvonz, #hg-reviewers, Alphare
Cc: Alphare, mercurial-patches
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mercurial-scm.org/pipermail/mercurial-patches/attachments/20220322/6e5548de/attachment-0002.html>