gyscos / cursive

A Text User Interface library for the Rust programming language
MIT License
4.17k stars 239 forks source link

[BUG] Building Cursive with `--features ansi` yields a future incompatibility warning due to nom v4.2.3 #748

Open cpdean opened 10 months ago

cpdean commented 10 months ago

Describe the bug Not a bug, but a compiler warning. Cursive built with ansi will print a warning that it will not build in a future version of Rust.

warning: the following packages contain code that will be rejected by a future version of Rust: nom v4.2.3

The slightly more detailed report suggests to upgrade to a newer version of nom.

To Reproduce

cargo build --features ansi
...
warning: the following packages contain code that will be rejected by a future version of Rust: nom v4.2.3

Expected behavior I would hope the latest version of Cursive would build on future versions of Rust.

Environment

macOS 13.5

uname -v
Darwin Kernel Version 22.6.0: Wed Jul  5 22:22:05 PDT 2023; root:xnu-8796.141.3~6/RELEASE_ARM64_T6000

cargo --version
cargo 1.70.0 (ec8a8a0ca 2023-04-25)

rustc --version
rustc 1.70.0 (90c541806 2023-05-31)
cpdean commented 10 months ago

Ah sorry I see this issue is actually coming from ansi-parser v0.8.0

christoph-heiss commented 10 months ago

Ah sorry I see this issue is actually coming from ansi-parser v0.8.0

Yep, this issue stems from nom 4.x pulled in transitively by ansi-parser 0.8. There is an open, draft merge request upstream, but neither the MR nor the repo seems to have seen any activity since ~1y ago.

Since I do have worked with nom a lot before, I'll might take a look at it. Besides that .. looking at the current usage of the ansi-parser crate, it might be even possible to get by without it. Only a small subset is used anyway, and parsing out these sequences isn't a lot of work (as can be evidently seen when looking at the source of the ansi-parser crate itself).

Should still be fine for some time though.

gyscos commented 9 months ago

I agree, I'm not very happy with the current state of ansi-parser. We're using a pretty old version (though there has been some development on the gitlab page since), and the logic should be simple enough to re-implement.

I'd still ideally make this as a separate crate (either a fork of ansi-parser or something new) than having the ansi implementation literally in cursive - makes it easier to re-use, and easy to switch out if needed.

correabuscar commented 3 weeks ago

This doesn't seem to be an issue anymore, it's now using:

ansi-parser v0.9.1
nom v7.1.3

fixed in commit f3bd7e96 mentioned in changelog via commit