Skip to content
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

ipcs: fix POSIX page; linux/ipcs, lsipc: add pages #12553

Merged
merged 6 commits into from May 7, 2024

Conversation

vitorhcl
Copy link
Member

  • The page(s) are in the correct platform directories: common, linux, osx, windows, sunos, android, etc.
  • The page(s) have at most 8 examples.
  • The page description(s) have links to documentation or a homepage.
  • The page(s) follow the content guidelines.
  • The PR title conforms to the recommended templates.
  • Version of the command being documented (if known): POSIX.1-2017 and util-linux 2.40-rc2

See #12354 (comment) details on the "See command1 for function1, command2 for function2" syntax proposal.

@vitorhcl vitorhcl requested a review from cyqsimon as a code owner March 25, 2024 19:24
@github-actions github-actions bot added new command Issues requesting creation of a new page. page edit Changes to an existing page(s). labels Mar 25, 2024
@tldr-bot

This comment has been minimized.

1 similar comment
@tldr-bot

This comment was marked as outdated.

@tldr-bot

This comment has been minimized.

@tldr-bot

This comment was marked as resolved.

@tldr-bot
Copy link

Hello! I've noticed something unusual when checking this PR:

  • The page pages/linux/ipcs.md already exists in the common directory.

Is this intended? If so, just ignore this comment. Otherwise, please double-check the commits.

Copy link
Member

@kbdharun kbdharun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, Thanks for your contribution.

See #12354 (comment) details on the "See command1 for function1, command2 for function2" syntax proposal.

We should probably document this in the style guide too.

@vitorhcl
Copy link
Member Author

See #12354 (comment) details on the "See command1 for function1, command2 for function2" syntax proposal.

We should probably document this in the style guide too.

Yeah, I was waiting for some more opinions because this replaces the already documented "See also: command1, for function1; command2 for function2" syntax, but sure, we should definitely document this.

@tldr-bot
Copy link

Hello! I've noticed something unusual when checking this PR:

  • The page pages/linux/ipcs.md already exists in the common directory.

Is this intended? If so, just ignore this comment. Otherwise, please double-check the commits.

@tldr-bot
Copy link

Hello! I've noticed something unusual when checking this PR:

  • The page pages/linux/ipcs.md already exists in the common directory.

Is this intended? If so, just ignore this comment. Otherwise, please double-check the commits.

@vitorhcl
Copy link
Member Author

@kbdharun @sebastiaanspeck @acuteenvy on second thought, I think the "See command1 for function1, command1 for function2" syntax is not ideal.

Introducing a different syntax for suggested pages can be worse for mixing suggestions with and without a description. Since @acuteenvy sees the ponctuation as the biggest problem, how about going with:

> See also: `command1` for function1, `command2`, `command3` for _function3_

@acuteenvy
Copy link
Member

acuteenvy commented Mar 30, 2024

What do you mean? There are no suggestions without a description in this PR.

@vitorhcl
Copy link
Member Author

What do you mean? There are no suggestions without a description in this PR.

I mean, if we want to standardize the syntax you suggested in the style guide, there may be sistuations like this, did you get the
"See also: command1 for function1, command2, command3 for function3" proposal? What do you think of using only this syntax?

@acuteenvy
Copy link
Member

there may be situations like this

Suggesting two pages with a description and one without looks weird to me nevertheless. I'd personally avoid this and either describe all suggestions or none.

@vitorhcl
Copy link
Member Author

vitorhcl commented Apr 5, 2024

Suggesting two pages with a description and one without looks weird to me nevertheless. I'd personally avoid this and either describe all suggestions or none.

I personally disagree, the "See also" line is so useful for suggest similar tools for both very similar tools, complementary tools or just similar to some extent, see #12582, where I connected several tools that belongs to a same category.

In pages that have like 3 to 5 suggestions, it may make sense to add a description for just the complementary one, for example, and don't add descriptions to very similar tools.

@kbdharun
Copy link
Member

kbdharun commented Apr 18, 2024

I agree with @acuteenvy here, having multiple descriptions referencing multiple pages/function looks weird to me as well. Just referencing the names of the multiple pages in See also line (i.e. See also: `htop`, `btop`. ) is fine with me too.

See also: command1 for function1, command2, command3 for function3

In this suggestion, the comma used can confuse users i.e. is command2 and command3 used for function3 or is command3 only used for function3.

@vitorhcl
Copy link
Member Author

vitorhcl commented Apr 18, 2024

In this suggestion, the comma used can confuse users i.e. is command2 and command3 used for function3 or is command3 only used for function3.

Yeah, I agree on that, my intention was to describe just the latter command.

@acuteenvy @kbdharun @sebastiaanspeck So I have 3 syntax proposals:

  1. Always use semi-colons. Examples:

See also: command1; command2

See also: command1 for function1; command2; command3 for function3

  1. Always use commas and not allow mixing with description vs. no description:

See also: command1, command2

See also: command1 for function1; command2 for function2; command3 for function3`

PS: I accidentally posted this comment on the another related PR, so I'm deleting this comment in the another PR.

@acuteenvy
Copy link
Member

Or 3. - as it currently is in this PR

See also: command1, command2.

See command1 for function1, command2 for function2, and command3 for function3.

@vitorhcl
Copy link
Member Author

vitorhcl commented Apr 19, 2024

Oh, I've accidentaly erased the other proposal, so:
4. Use "See also" when there is no description and "See for" when there is a description, as @acuteenvy originally suggested:

See also: command1, command2.

See command1 for function1, command2 for function2, and command3 for function3.

Edit: Personally, I prefer the other 3 because of the eventual client usage I've talked about in my previous comment.

What do you guys prefer? Or do you have a better idea?

@vitorhcl
Copy link
Member Author

@acuteenvy @kbdharun @sebastiaanspeck any opinion on these 4 proposals or a new proposal?

@kbdharun
Copy link
Member

any opinion on these 4 proposals or a new proposal?

Just now stumbled upon this ping while checking the notifications, IMO the 3rd one that we currently use LGTM. Using semi-colons in the 1st and 2nd proposals looks odd compared to using commas. The fourth proposal changes the syntax so the 2nd "see also" line won't be rendered differently by clients implementing styling for this.

pages/linux/lsipc.md Outdated Show resolved Hide resolved
pages/linux/ipcs.md Outdated Show resolved Hide resolved
pages/linux/lsipc.md Outdated Show resolved Hide resolved
Co-authored-by: K.B.Dharun Krishna <[email protected]>
@tldr-bot
Copy link

tldr-bot commented May 6, 2024

Hello! I've noticed something unusual when checking this PR:

  • The page pages/linux/ipcs.md already exists in the common directory.

Is this intended? If so, just ignore this comment. Otherwise, please double-check the commits.

vitorhcl and others added 2 commits May 5, 2024 23:59
Co-authored-by: K.B.Dharun Krishna <[email protected]>
Co-authored-by: K.B.Dharun Krishna <[email protected]>
@tldr-bot

This comment was marked as duplicate.

1 similar comment
@tldr-bot

This comment was marked as duplicate.

@sebastiaanspeck sebastiaanspeck merged commit b6ac034 into tldr-pages:main May 7, 2024
4 checks passed
@vitorhcl vitorhcl deleted the lsipc-ipcs-add-page branch May 7, 2024 05:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new command Issues requesting creation of a new page. page edit Changes to an existing page(s).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants