Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CLIENT_SPECIFICATION.md: Write initial client specification #2706
CLIENT_SPECIFICATION.md: Write initial client specification #2706
Changes from 5 commits
4e51358
e60801d
f0f94e6
eb8e2e4
e626bf4
7d67830
2e3641d
9206c25
718f8e7
e573fbf
bba9222
f22c233
794cbd3
86dd357
3841a80
7f4a5ee
1bd331d
cde22bd
e2a764e
3eea772
12af654
4ae2b29
cab265e
566b0f9
4715e71
83c0915
4a2c2bf
71db6de
0124246
f200f1b
823cfa4
af6afa2
551db32
73f2bc8
2077747
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe we need
-h, --help
here too?BTW options are typically written in reverse order:
-v, --version
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how about
--list
,-l
? This way it is similar tols -l
(one per line), and shorter to type.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hrm, you raise a good point. What do you think, @agnivade / @pxgamer / @waldyrious?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this was because the node client uses
--list-all
🤔 I think--list
is used to show pages for the current platform.See documentation here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think
--list-all
is rather confusing, actually. It implies that all pages are listed, but in reality it's only the pages in the current platform & common that are listed.Perhaps we can disambiguate this somehow?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally I don't see any value in listing every possible page, crossplatform (which I would assume
--list-all
does as @jedahan pointed out) because that means we should print out paths to disambiguate pages covering same tool on different platforms. That essentially puts us in business of re-implementingls -R
. If user really wants that they should be able to get cache location somehow and do that on they own.So my vote goes to
-l, --list
to get list of pages for assumed/specified platform.OTOH discussion about list formatting is (sorry but I have to say it) aiming at completely wrong target. It simply does NOT matter at all how it will look and there is good background reason for that. Take our beloved
ls
for example. Default output ofls
is columned or is it?! Pipe that throughcat/less
and you'll get one-per-line list. I'm too lazy to dig out proper POSIX/TAOUP/whatever spec/recommendation so rule of thumb instead:Output of first class citizen *NIX CLI tool:
if
STDOUT
is NOTTTY
if
STDOUT
is aTTY
Can we just replace blah blah CLI tool with
tldr
client and put it inside spec? 😉There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still think if people want to do something that technical, they could either just turn to the cache directory and get anything they need there, or, they should use the
-t
/--tty
flag or-n
/--noformat
or some such. Because if people pipe their output intoless
, they probably don't want to automatically get no columns or colors. If you use a flag, the user can decide, if you don't, some users are going to request it as a feature.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pepa65
And that's like only thing I agree with. 😞
Calling list manipulation with standard cli tools technical is bit far fetched if you ask me; after all isn't one of selling points of
tldr
pages is to get people to that level more easily? 🙃What so technical in grepping large lists? 🤔
I'm very curious to see example of tool implementing given option. I never seen something like that and I highly doubt I ever will; having in mind it is not considered standard:
(
tty
used bygdb
is not switch but an option with an argument: https://manpages.ubuntu.com/manpages/precise/en/man1/gdb.1.html)Sorry but that's just your opinion while common practice teaches us exact opposite. I may or may not agree with you but you are actually arguing with facts:
So, yeah you are right, people like ansi colors but like and expect by default are two different things. By default colors are off. Which brings me to the next thing you said:
Again, you are going against common practice established by *NIX cli tools. Formatting is off by default if output is tty. If you really want to retain it you need to explicitly request tool generating output to do so. And it makes perfect sense why you need extra step for colors/formatting to be outputted/preserved: not every tool you may use to process output understands and/or is able to strip out ansi escapes! If user is positive that his tool of choice (e.g.
less
) is capable of doing so, ok, you have choice to do--color=always
,--color
,--pretty
, you name it...In conclusion - to actually address your concern we should probably support
--pretty
option that would force colors and general formatting being preserved when output is nottty
. I'm ok with that and FWIW you have my 👍 there but in form of recommendation, such thing shouldn't be a MUST but MAY instead:stdout is TTY
- Generate formatted output (optional)stdout is NOT TTY and --pretty
- Preserve formatting (optional)stdout is NOT TTY
- Generate\n
separated list (mandatory)That being said, please try to align your proposals with established CLI tools design practices and user expectations. TBH I understand your motivation and would sometimes give everything for a chance to rewrite history of our discipline but that's simply not possible.
tldr
clients are not first tools of such class and less exotic they are more successful and useful they'll become!There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pepa65
Yes and no 🤔
Sure it is verbose, I personally don't like it too but...
).
OTOH lets compare this with what
man
does (I like to considertldr
client asman
for humans same way ashttpie
iscurl
for humans; correct me if my understanding is wrongIn order to get all
man
pages you need to run:where
.
is regex meaning everything. If you want to narrow that down using sections:lists only manpages inside 1st section.
If you now swap
-k .
with-l
and-s1
with-p <platform>
you'll get original form I proposed:You could OFC argue that manpage listing I just described has mandatory search param while tldr listing does not thus putting us in better position by allowing what you suggested. Yeah, that's true but might be also an indication that we are doing something wrong⚠️
Maybe right way to do the listing is to copy good ol' man? 🙃
@jedahan
Could not support you more in that consistency effort 💯
@sbrl
I wanted to do some wildcard like thingie and ended up doing lousy job 🤦♂️
I know it isn't constructive at all but this whole thing with what should list (almost) everything print is painfully convoluted and more we evolve it less value I see in it. Can someone ELI5 in what particular case user wants to print every or almost every possible command out apart from pure curiosity and explorer's spirit?
Aren't the real life usecases:
(step zero: you know what you want to do)
Which isn't even mentioned in the spec itself!⚠️
In both cases you might be searching answers for a platform you are currently not working on so you'll add
--platform <target_platform>
too.I mean this is exactly what
man
supports also:while listing all pages is available by means of empty query but it certainly isn't something that is endorsed by having dedicated option like
--list-(almost/maybe/platform/common)-all
🙃There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright then, let's go with default being current platform and having a reserved platform
all
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great discussion! I'll update the spec to reflect this.
Met me know if I miss anything :P