search

colored-rs / colored

(Rust) Coloring terminal so simple you already know how to do it !
Mozilla Public License 2.0
1.73k stars 83 forks source link

Switch from `winapi` to `windows-sys` #143

Closed LingMan closed 1 year ago

LingMan commented 1 year ago

winapi is being phased out across the ecosystem in favor of windows-sys, which is maintained by Microsoft. Let's follow suit so dependents don't have to compile both.

LingMan commented 1 year ago

Notably is-terminal, which you plan to potentially use as a replacement for atty for now, also uses windows-sys.

hwittenborn commented 1 year ago

Where did you see that winapi is being phased out @LingMan? I'm more than fine getting this merged in if that's the case, I just didn't see anything on it after a quick Google.

LingMan commented 1 year ago

There's no expressed consensus written down anywhere, if that's what you were looking for. Just more and more crates in my dependency trees made the switch to the point that only a small number still pull in winapi.

LingMan commented 1 year ago

@hwittenborn Could you merge this for v2.0.3 which you apparently plan to release soon? Since colored now depends on is-terminal it pulls in windows-sys and winapi all by itself.

hwittenborn commented 1 year ago

Yeah that's fine @LingMan, I definitely don't want the double dependencies floating around.

I'm just going to wait for #146 to get merged so the updated CI can be ran, and then I'll go ahead and get this merged in.

hwittenborn commented 1 year ago

Hey I know I said I'd get this merged after #146 got merged in @LingMan, but I'm wanting to set up some Windows CI as well. I just don't want to get in anything that I can't personally guarantee works.

I know I pushed that version PR, feel free to delete it when you need to merge in or anything. I should be able to have the Windows CI going in a bit.

hwittenborn commented 1 year ago

Alright if you could update master I can go ahead and get this merged in @LingMan :)

hwittenborn commented 1 year ago

Thanks for the PR!