search

mitsuhiko / similar

A high level diffing library for rust based on diffs
https://insta.rs/similar
Apache License 2.0
974 stars 32 forks source link

Update `bstr` to 1.x #40

Closed dbanty closed 6 months ago

dbanty commented 2 years ago

The only impacting breaking change seems to be that enabling unicode support requires enabling std.

mitsuhiko commented 2 years ago

Need to figure out why this does not run CI. Created a second PR for this here to get CI to run: #41

This likely will mean that bstr support will have to move the MSRV up significantly for when bstr is used.

mitsuhiko commented 2 years ago

Looks like I won't be able to merge this for a while. I am using the bstr feature in insta via similar still and I want to at least try to support older versions of rustc for quite a while longer since it's a testing tool. Not sure yet how to deal with this.

dbanty commented 2 years ago

Gotcha, no worries, I was just trying to remove a duplicate dependency from something else that consumes this. Thanks for figuring that out!

mitsuhiko commented 2 years ago

Yeah, that makes a lot of sense. I'm going to figure out what to do here. Made a meta issue over here: https://github.com/mitsuhiko/insta/issues/289

BurntSushi commented 2 years ago

The only impacting breaking change seems to be that enabling unicode support requires enabling std.

Just a note that, previously, I believe enabling unicode implicitly enabled std via lazy_static. bstr 1.0 was going to do the same with once_cell, but then I realized it would be a backcompat hazard. So I required folks to be explicit about it. That way, I can remove std as a dependency of the unicode feature in the future. (And I do expect this to happen at some point by dropping once_cell.)

Here's the commit where I made the change that has some more details: https://github.com/BurntSushi/bstr/commit/72024a801c261bf351e88172a66f6a00ef4e3b86

smoelius commented 7 months ago

I would like to respectfully bump this.

As mentioned in https://github.com/mitsuhiko/insta/issues/289, merging this PR would require similar's MSRV to be atleast 1.60 (April 2022).

Do you know of similar dependents that require a compiler that old? If not, would it be possible to move forward with this?

mitsuhiko commented 7 months ago

Insta (and thus similar) is used by libraries that target 1.56. syn being an example.

smoelius commented 7 months ago

Should that matter, though, since insta is only a dev-dependency of syn?

EDIT: FWIW, I ran this in the syn repository:

cargo msrv -- cargo check --tests --all-features

And the result was 1.62.1.