Replace term with anstream?

[Pat-Lafon]

Not a high priority but I noticed that a more up-to-date Cargo.lock brings in a second version of bitflags because of the dependency on term. This got me looking into this crate which generally doesn't seem to be well maintained(Stebalien/term#93) and has since been replaced in the ecosystem afaik(Termcolor was the original replacement, but maybe people are moving away from that for anstream rust-lang/cargo#12751).

It would probably benefit Lalrpop to refactor the cli to use anstream instead of term and it might even clean up that area of the code base a little bit with it's hacks like FakeTerminal

[dburgener]

I started briefly poking at this. Part of the challenge here is that our usage of term is also tied into our ASCII canvas dependency (https://github.com/nikomatsakis/ascii-canvas). Ascii canvas is also written by Niko like lalrpop, and it looks like it was originally part of lalrpop, but since it had independent use, he spun it out into its own separate crate. The term dependency is used across the API between lalrpop and ASCII canvas. Probably the strategy here needs to be to first go update ASCII canvas (I imagine we can ping Niko and get that change in even though he's presumably not actively managing the project). Then that could get released as a breaking change, since the API would move away from term. Then we can make a change here to update our ASCII canvas version while also dropping the term dependency.

I might work on this at some point, but it seems like a pretty large task, so I'm not able to commit the time right now.

[nikomatsakis]

Whoops. I'd happily pull ascii-canvas into lalrpop org, I doubt it has many independent users.

…

On Sun, May 19, 2024, at 5:27 PM, Daniel Burgener wrote: I started briefly poking at this. Part of the challenge here is that our usage of term is also tied into our ASCII canvas dependency (https://github.com/nikomatsakis/ascii-canvas). Ascii canvas is also written by Niko like lalrpop, and it looks like it was originally part of lalrpop, but since it had independent use, he spun it out into its own separate crate. The term dependency is used across the API between lalrpop and ASCII canvas. Probably the strategy here needs to be to first go update ASCII canvas (I imagine we can ping Niko and get that change in even though he's presumably not actively managing the project). Then that could get released as a breaking change, since the API would move away from term. Then we can make a change here to update our ASCII canvas version while also dropping the term dependency. I might work on this at some point, but it seems like a pretty large task, so I'm not able to commit the time right now. — Reply to this email directly, view it on GitHub <#856 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABF4ZXQWC5NSGMEF6WWXMLZDEKLXAVCNFSM6AAAAAA7PG4NUCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMJZGM3DKMBSHE>. You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[Pat-Lafon]

I started briefly poking at this. Part of the challenge here is that our usage of term is also tied into our ASCII canvas dependency (https://github.com/nikomatsakis/ascii-canvas). Ascii canvas is also written by Niko like lalrpop, and it looks like it was originally part of lalrpop, but since it had independent use, he spun it out into its own separate crate. The term dependency is used across the API between lalrpop and ASCII canvas. Probably the strategy here needs to be to first go update ASCII canvas (I imagine we can ping Niko and get that change in even though he's presumably not actively managing the project). Then that could get released as a breaking change, since the API would move away from term. Then we can make a change here to update our ASCII canvas version while also dropping the term dependency.

I might work on this at some point, but it seems like a pretty large task, so I'm not able to commit the time right now.

Maybe not too bad? I threw together an "it-compiles" version over the last one/two hours(https://github.com/Pat-Lafon/lalrpop/tree/anstream)... though most of that was figuring out how the functionality was split over a couple of crates. Unfortunately I don't think colors are that easy to test(atleast neither of the crates seem to check for this)? It's probably a long tail of checking for differences in implementation.

[dburgener]

Unfortunately I don't think colors are that easy to test(atleast neither of the crates seem to check for this)? It's probably a long tail of checking for differences in implementation.

Can you elaborate about what is concerning you there? I agree that it's challenging to test for changes in terminal coloring, particularly in terms of corner cases, cross platform support, different environments etc. But most of the color bugs aren't really all that bad. Even if we entirely lose color in some situation, or worse add color markers in an area where they won't be interpreted, that's not a show-stopper sort of bug.

My inclination would be to do as much manual testing as we reasonably can, and figure that if we missed something it won't be all that huge a problem for users, and we should be able to fix it pretty easily in a point release. Particularly if we were to get this in the for the next release which is breaking, then users would be manually upgrading anyways, and have an opportunity to see if things break during that, rather than have them break on a point release if we mess something up.

In terms of ascii-canvas users, crates.io has the handy reverse dependencies search, and it shows that lalrpop is the only user: https://crates.io/crates/ascii-canvas/reverse_dependencies

I would definitely advocate for pulling ascii-canvas into the lalrpop org. Github's org migration features are really good, and maintain previous links as a redirect, so doing org migrations tends to be pretty seamless for users, as long as you use the official github interface rather than just push a new copy or something.

[youknowone]

If making test cases is possible, cargo insta is useful to make snapshot tests
https://crates.io/crates/cargo-insta

[nikomatsakis]

I have transferred ascii-canvas to the lalrpop org-- you all should have access

[dburgener]

Thanks!

@Pat-Lafon do you want to create PRs for your code above? I'm happy to review and test.

I'd like it if we could do a release in the somewhat near future to get out the recent bug fixes (particularly #898, but also #896), but my preference would be if this could also get included.