feat(rust): paginate help texts

[i-b-o-t]

using $PAGER if set, defaulting to less and falling back to more or no pagination if no pager is available.

Addresses #3434

Checks

• All commits in this Pull Request are signed and Verified by Github.
• All commits in this Pull Request follow the Ockam commit message convention.
• I accept the Ockam Community Code of Conduct.
• I have accepted the Ockam Contributor License Agreement by adding my Git/Github details in a row at the end of the CONTRIBUTORS.csv file in a separate pull request to the build-trust/ockam-contributors repository. The easiest way to do this is to edit the CONTRIBUTORS.csv file in the github web UI and create a PR, this will mark the commit as verified.

[mrinalwadhwa]

@i-b-o-t thank you for spending time on this.
Use of less worked like a charm, nice!

A few quick things:

1. Could you run cargo fmt and cargo clippy and fix any problems they report
2. Our commit messages are all lower case so please amend the commit message to feat(rust): paginate help texts

After that, I think we can merge this.

[i-b-o-t]

Sure, I can do that.

Also, I noticed that the exit status of the process should be either 0 or 2 depending on whether the help text is requested by the user explicitly or shown as a consequence of a usage error. By the same logic, in case no pagination is available, the output should go to stdout or stderr streams respectively.

Also, checking whether the pager is less should better be done via path::file_name in case the environment variable contains a full path.

So give me a minute to take care of that.

[i-b-o-t]

In individual commits for easy reviewing. The first one is still the same, except for rewording its log entry to lowercase. Rebasing with --autosquash will turn them all into a single pretty one automagically, as you most probably know.

[i-b-o-t]

$ PAGER=false ./target/debug/ockam -h 1>/dev/null && echo OK
OK
$ PAGER=false ./target/debug/ockam 2>/dev/null || echo OK
OK
$ cat mock/swallow 
#!/bin/sh

$ PAGER=./mock/swallow ./target/debug/ockam -h && echo OK
OK
$ PAGER=./mock/swallow ./target/debug/ockam || echo OK
OK
$ cat mock/less 
#!/bin/sh

if test "${LESS}" = "-F"; then
	echo OK
fi
$ PAGER=./mock/less ./target/debug/ockam 
OK

[adrianbenavides]

Nice job @i-b-o-t! It's pretty much there but I've left some comments with potential improvements. Thanks for taking the time with this 🙏

[i-b-o-t]

💟 Thanks for your review! Please don't get too negative of an impression from me disagreeing in many places and let's not obscure the fact that it did raise valid points.

[i-b-o-t]

Behaves just like Git now, AFAICT rebased and ready.

[adrianbenavides]

💟 Thanks for your review! Please don't get too negative of an impression from me disagreeing in many places and let's not obscure the fact that it did raise valid points.

No problem! That's the whole point of doing code reviews right? We are here to learn from each other 👍

[adrianbenavides]

Hey @i-b-o-t , I was taking another look at the PR and I just wanted to clarify something. The PR looks great, it works as expected and we could merge it as is. All my last comments are just questions that would help me understand better your solution and small details that could be improved, but they won't block the PR from merging.

That being said, feel free to mark the comments as resolved. We can tackle whatever is left in a separate PR.

And thanks again for taking the time with this issue, it ended up being more tricky than I initially anticipated!

[i-b-o-t]

Hey @adrianbenavides , thanks for giving it another chance - being able to take a step back and reconsider one's own position shows character! Defending it comes with wholehearted commitment, I know, nothing wrong with that as long as we realize when to hold on and take a break, a deep breath, another look or perspective into consideration.

The simplest solutions is not always immediately obvious.

I do think that Occam's Razor is a fine analogy to problem solving in software development and given the scope and constants that constrain us here and now, the implementation in a nutshell is as simple as it gets.

Once we widen the scope (e.g. by introducing further command line options, changing the public interface, as we both have suggested above) or find a way to redefine the constants (e.g. by requesting additional Clap API), there is always room for improvement piecing the puzzle together in a coherent fashion. However, in order for these efforts to be both manageable and motivating, I think it is a good idea to chop things up into individual working steps and then working that plan without too much deviation. Of course there are exceptions, like something being completely infeasible, but we don't have that case here. Also, there is almost always some nits to pick on; doesn't mean we should just because we could (invert priorities).

The PR looks great, it works as expected and we could merge it as is

Just pushed the squashed commit.

EDIT: To complete my philosophical sermon and add another quote

That's the whole point of doing code reviews right? We are here to learn from each other

I agree, plus hopefully merging better code than we would without.

From that angle, nitpicking certainly has its place in reviews, as it can mean teaching each other (often ideally generalizable) details to be aware of, but it also bears the risk of wasting time and inverting priorities, so there's a pair of anti-forces that need to balanced.

[adrianbenavides]

Thanks for the message @i-b-o-t , I really appreciate it 🙏 . One last thing: we require the commits to be signed. Let me know if you want me to sign it or you want to do it yourself. It's a one time operation and it's pretty quick to set up.

[adrianbenavides]

Shouldn't I see an error if they were not. Oh, or does it also need that "Signed off by" line in the commit message?

That's not needed. Looks like your commit is signed but can't be verified. I think you have to upload your public key to your githu account for the verification to work.

[adrianbenavides]

Let's get this merged! Thanks again @i-b-o-t, this was a great contribution! 🚀