One of the more foundational crates found in the dependency tree of a lot of
rust programs, because both clap and env_logger pull this in.

In my review I've fully read the source code and can confirm that I fully
understand what's happening in here. The unix and hermit targets are
extremely straight-forward. As for the windows target, that's a bit more
complicated, but still manageable in the end. Windows doesn't have a clear
API for determining whether something is a (pseudo) TTY, so the heuristics
provided by this crate are as good as it's going to get.

This crate has quite a few unsafe code sections, but that's sometimes
required for providing a safe interface. In this case, we need it because the
underlying functions for unix (libc) are unsafe, and the same applies to a
bunch of winapi functions used in the heuristics for windows.

The bits of unsafe code that's not just wrapping an unsafe function provided
by another library are all in the windows heuristics, and involved
provisioning buffers that winapi calls can write info back into and some
pointer magic. While I couldn't spot an issue with this, a look into the issue
tracker revealed that other's have. The buffer creation on the heap is not
necessarily aligned properly, meaning that there's a possible soundness issue
on windows targets here.

Last but not least, we have to talk about maintenance and and alternatives:
The soundness issue mentioned above has been known for over a year, with a fix
first pushed to a PR shortly after. Even though there's been reviews from
well-known rustaceans, the author hasn't merged this PR yet, and in general
there hasn't been any relevant activity for a while.

An alternative implementation that has been derived from atty is
is-terminal, which has taken this into consideration and has taken over the
rest of the implementation from here, with a slightly different API. They're
also switching the underlying implementations around, reducing the amount of
unsafe.

As for why I'm still rating this as positive: With the exception of the
potential unsoundness bug on windows, this crate is still okay-ish to be used.
Also: Getting rid of it soon is not realistic, because it's in the dependency
tree of quite a few widely-used crates.

To summarize: Widely used, foundational crate. Except for on windows, this is
perfectly fine, on windows there's an unsoundness bug, and there has not been
an update or other activity from upstream. is-terminal is a good
alternative, which was derived from this crate, but includes the fix for the
unsoundess bug.

As the README claims, this locks files using the fcntl libc method, and it
provides locks for both read and write access. I've read the man pages for
that thoroughly and compared it to the implementation of this crate, and at
least the C part of it is nearly perfect. I would have named some things
differently, but aside of that, this part is good as it stands. It always
locks the whole file, uses the proper flags and in general looks good. As for
the rust part.. well, that's where the problems begin. The Drop impl does
unlock the file, but ignores the result of the unlocking (it calls .is_ok() on
the Result and ignores the return value, which is an antipattern also present
in all of the test cases). When aquiring an exclusive lock, it disables read
access to the file, causing bad fd errors when trying to read anyway. Last but
not least, this crate hasn't been updated for the 2018 edition, has tons of
warning and is using the deprecated and unmaintained gcc crate. These problems
are easily fixable, and where I'm using the crate, I'm using my fork which has
all these problems fixed, but considering that the other 3 MRs of the last
year haven't been fixed either, I have my doubts about this still being
maintained. If upstream continues to not respond, I'll fork this and update my
review with an alternative accordingly.

Following up my review of atty, this is the alternative that I'm recommending.
The crate is well-maintained and has the soundness problem from atty fixed.
The dependencies have been changed around a bit, with rustix replacing libc,
and windows-sys replacing win-api. This means that there's a bit less unsafe
code in the library here as well.

The API surface changed a bit as well, but I'd claim that the new API is more
sensible and allows usage on any file descriptor instead of just
stdin/stdout/stderr.

Finally, this has one additional dependency, io-lifetimes, by the same
author. I have not reviewed the implementation, but it's been merged into Rust
itself and stabilized with 1.63, so I am not worrying about it too much.

Short and concise implementation. A little code that's technically unsafe, for
lowercasing ASCII, as Rust's to_lowercase() impl is slower because it's
unicode aware. As it doesn't have to handle unicode where it's used, it is
still sound.

Aside of it's own code, there is unsafe in the deps too: If deunicode is
fine, then this is too. I didn't invest enough time in deunicode to make a
final judgement about it, but others have so it's probably okay.

Useful middleware, solid implementation. I'm honestly surprised that I
haven't found any alternatives, considering this is such a basic feature.

Follow-up from my previous review for version 0.7.2: The sound use of unsafe has now gotten a test case to show that it's actually working as expected with exotic characters, and the Tokens::optional_err function I was missing last time is something I've contributed since. Still very happy with yap, and looking forward to seeing this used more.

For a more detailed review, read my previous coverage of 0.7.2.

Simple enough parser combinator library that comes with most of the bells and
whistles I was looking for. I've noticed a few minor things that could use
improving, but nothing critical.

First of all, why did I review this? I implemented parsing a specific HTTP
auth header credential, which is non-trivial to do with just string splitting.
I had gathered some experience with yap during last years Advent of Code,
and I'm relatively happy with the implementation now. The only thing I was
missing is something like Tokens::optional that was based on Result
instead of Option. I'll make sure to contribute that later though.

As for problems I've found while reviewing this crate:

• There's one sound use of unsafe, that could probably be avoided, just for
  good measure. It's about going from a str to the next char. It
  increases a counter by one, and then by another one until it's at a char
  boundary. This is explicitly checked before it then does
  str::get_unchecked, which is the unsafe call. It seems sound to me, but
  without tests this isn't a great look.

• The formatting is a bit off in some places. A few extra spaces, nothing
  big, just made reviewing a bit more of a chore.

• The logos dev-dependency doesn't seem to be used anywhere. As logos is
  quite wide-spread, it being included in yap is not much of a supply chain
  safety loss, but when it's not used it should probably be dropped anyway.

Things I specifically liked:

• The documentation is amazing. Every single thing one could hope for has a
  doc comment, and the documentation really goes above and beyond to
  demonstrate how yap can be used.

• Leveraging iterators was a very good choice. Using yap feels super
  natural, and even without any prior parser-combinator experience, I was
  able to quickly get started. Not using regular expressions and string
  splitting feels really good.

In general, I'm extremely happy with yap, and even happier now that I've done
my due diligence with regards to supply chain security. Thanks to the authors.

Simple enough parser combinator library that comes with most of the bells and
whistles I was looking for. I've noticed a few minor things that could use
improving, but nothing critical.

First of all, why did I review this? I implemented parsing a specific HTTP
auth header credential, which is non-trivial to do with just string splitting.
I had gathered some experience with yap during last years Advent of Code,
and I'm relatively happy with the implementation now. The only thing I was
missing is something like Tokens::optional that was based on Result
instead of Option. I'll make sure to contribute that later though.

As for problems I've found while reviewing this crate:

• There's one sound use of unsafe, that could probably be avoided, just for
  good measure. It's about going from a str to the next char. It
  increases a counter by one, and then by another one until it's at a char
  boundary. This is explicitly checked before it then does
  str::get_unchecked, which is the unsafe call. It seems sound to me, but
  without tests this isn't a great look.

• The formatting is a bit off in some places. A few extra spaces, nothing
  big, just made reviewing a bit more of a chore.

• A previous version of this review claimed that the logos dev-dependency
  wasn't used anywhere. That's true for the releases on crates.io, but the
  repository contains examples, where logos is actually used.

Things I specifically liked:

• The documentation is amazing. Every single thing one could hope for has a
  doc comment, and the documentation really goes above and beyond to
  demonstrate how yap can be used.

• Leveraging iterators was a very good choice. Using yap feels super
  natural, and even without any prior parser-combinator experience, I was
  able to quickly get started. Not using regular expressions and string
  splitting feels really good.

In general, I'm extremely happy with yap, and even happier now that I've done
my due diligence with regards to supply chain security. Thanks to the authors.

I've authored this and we've used it in production. We've had one overflow
bug for a while, but we've not experienced any issues after fixing that one.

We had multiple people look over it by now, and we've not found any more
issues. The keys generated by this work fine with yggdrasil-go, and after
fixing that one bug we've not experienced any incompatibilites anymore.