Fix some color-bleed for non-pixel perfect fonts (fixes #72)

[rivy]

Here's the PR I promised.

I've separated it into three commits: 1) the repair commit (swatch/checkerboard resize) [5f7bf24], 2) removal of vertical padding between swatches [66eba3f], 3) adding vertical padding under outer function control to allow for better visual symmetry of output (otherwise, there's extra padding either at the top or bottom) [462b00a].

To get a one-of vertical padding line at the top of the output, I used an atomic boolean as state. It feels like a hack, but I'm not sure of a better "safe"-function way. Adding an iterator to the functions interface seems like a less desirable, more heavy-handed approach.

So, I left the commits separate for easier modification to a final result.

I think the final result looks good. And the atomic, while a bit hack-ish, should be lock-free and speedy per the documentation.

Thoughts?

[sharkdp]

Thank you for your contribution (and top quality commit messages / documentation)!

I really like how the new output looks like and that we can fit more swatches on one screen.

However, I'm not a fan of the global mutable (atomic) variable in the implementation.

I have a solution in mind that would also improve a few other things, but it is a bit more involved. I can outline it below, but we don't have to do this here. I could work on that in a follow-up PR (unless you want to look into it).

The idea would be:

• Introduce a new Output struct that includes the show_color and show_color_tty functions from src/cli/command/show.rs as well as the handle to standard output (the out: &mut dyn Write parameter that we pass around everywhere).
• Then, instead of passing out: &mut dyn Write into all of the commands, we pass a output: &mut Output handle that every command can use (src/cli/commands/traits.rs) to call output.show_color(…) instead of show_color(out, …). If a command needs direct access to the output handle, it can still write!(…) to output.handle.

This way, we could keep a is_first_color: bool in the Output struct that would be set to false after the first call to Output::show_color. The rest would be similar as in your implementation.

[rivy]

Sure, I'll take a stab at it this weekend!

[rivy]

I've rebased off current master and put some work into refactoring, replacing the atomics.

I implemented the structure at a low level within the module, at the level of the commands. The implementation passes an IO handle into the commands from the CLI router and builds Output within the commands which need it. It could be moved within the hierarchy if you are considering it as a solution to "improve other things" and would work better at a higher level. Let me know and I can move it up.

I'll also squash out the atomics commit when finishing up the PR.

The first-pass is at 3c61fd4 .

If it's a reasonable start, I'll add it to this PR and continue work from that point.

[sharkdp]

Thank you for the updates. Yes, this looks very promising!

Instead of creating new Output objects all over the place, I would try to replace the out: &mut dyn Write parameters in all commands with a new output: &mut Output parameter.

[rivy]

Alright, I've pull up the creation of Output into the main CLI routine before calling the sub-commands.

Let me know what you think...

In the dev-process, I was testing on a Windows 7 machine which prompted me to work on a change to allow pastel to function for non-ANSI-escape machines (see [4ebf999f and 48d392a]). I think the functionality delivered by pastel can be valuable even if the output isn't displayed in color.

[sharkdp]

Thank you very much. The updates look great! Only 2-3 minor comments.

Let me know if you need help in resolving the merge conflicts.

[rivy]

Yipes, you were quicker than I expected. I was going to squash the commits and test them this afternoon before offering it for merge (I should have pushed it to a temporary branch for storage). The "fixup!" commit will try to merge with the commit two prior if you rebase anything around it. I hope that's not a problem.

[sharkdp]

I tested your changes locally and it all looked good to me. I was a big fan of rebasing/squasing/cleaning-up once, but sometimes I also appreciate the historically correct version. So don't worry about it.

Do you think there is anything else that needs cleaning up / testing?

[rivy]

Nope, if you like the history then it's good from my end.

I will be opening a PR today or tomorrow with some small changes to allow pastel to function (with some small lmitations) for non-color-capable consoles, such as Windows 7.

I mentioned it (with the related commits) briefly in a prior post.

[sharkdp]

I will be opening a PR today or tomorrow with some small changes to allow pastel to function (with some small lmitations) for non-color-capable consoles, such as Windows 7.

sounds great!