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.

Sign up for GitHub

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

Jump to bottom

man(1) clarify searching + minor nits #1438

Closed
wants to merge 1 commit into from
Closed

man(1) clarify searching + minor nits #1438

wants to merge 1 commit into from

Conversation

concussious
Copy link
Contributor

@concussious concussious commented Sep 28, 2024 •
edited
Loading

Attempt to clarify search while maintaining terseness. Inspired by conversations on community discord!

Unresolved:

  1. I'm having trouble understanding example 3, it doesn't find anything on my sparse but inhabited boxen.
  2. logic preventing incompatible flags from being used together (e.g. man -Kk, seems to start around line 600) is not working on CURRENT. I tried playing with it but I don't understand it enough yet.

Cc @tetlowgm

@concussious concussious force-pushed the man.1 branch 2 times, most recently from 7464aa7 to 5f1b617 Compare September 28, 2024 20:53
@concussious
Copy link
Contributor Author

concussious commented Sep 28, 2024 •
edited
Loading

I think these changes should be sensible and non-controversial, but perhaps controversially I'd love to:

  1. Use A a B b sorting like other manuals
  2. s/online/local/. Online is traditional language since ancient times, and I feel tenderly about that, but I'm concerned that nobody who didn't already know knows what that means.

@concussious concussious changed the title man.1: clarify searching + minor nits man(1) clarify searching + minor nits Sep 29, 2024
@concussious concussious force-pushed the man.1 branch 3 times, most recently from 6eba44e to 78e96d0 Compare September 29, 2024 05:54
elliejs
elliejs approved these changes Sep 30, 2024
View reviewed changes
Copy link

@elliejs elliejs left a comment

Choose a reason for hiding this comment

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

Looks great! I love these incremental improvements. I wrote a few tiny nitpicks but for the most part I think this looks splendid.

This PR works on my machine and the build failures appear to be due to the build system being throttled, not a syntax error.

usr.bin/man/man.1 Show resolved Hide resolved
usr.bin/man/man.1 Outdated Show resolved Hide resolved
usr.bin/man/man.sh Outdated Show resolved Hide resolved
usr.bin/man/man.sh Show resolved Hide resolved
@concussious
Copy link
Contributor Author

Thanks for the review! I worked egrep back into -K description.

@concussious concussious force-pushed the man.1 branch 2 times, most recently from 3234f6c to 22d7ed7 Compare October 1, 2024 01:48
bsdimp
bsdimp reviewed Oct 4, 2024
View reviewed changes
usr.bin/man/man.1 Outdated Show resolved Hide resolved
@bsdimp
Copy link
Member

bsdimp commented Oct 4, 2024

this may need other changes, but can we not change whitespace on the man pages.

@bsdimp bsdimp self-assigned this Oct 4, 2024
@concussious
Copy link
Contributor Author

I think this would be a major positive feature for 14.2. This is the introduction to manual pages, and the current explanations for the search features are only useful for people who already know what they mean.

@concussious
Copy link
Contributor Author

concussious commented Nov 4, 2024 •
edited
Loading

Last push removes prompts from examples for triple click copy/paste. Subsequent pushes just tweaking commit message.

@concussious concussious force-pushed the man.1 branch 4 times, most recently from 7b73d9c to 55d7b77 Compare November 7, 2024 17:09
jlduran
jlduran reviewed Nov 13, 2024
View reviewed changes
Copy link
Member

@jlduran jlduran left a comment

Choose a reason for hiding this comment

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

Good, overall this is an improvement.

I have expressed the reasons why I do not agree with some of the proposed changes.

Thank you!

usr.bin/man/man.1 Outdated Show resolved Hide resolved
usr.bin/man/man.1 Show resolved Hide resolved
usr.bin/man/man.1 Show resolved Hide resolved
usr.bin/man/man.1 Outdated Show resolved Hide resolved
@concussious
Copy link
Contributor Author

concussious commented Nov 14, 2024 •
edited
Loading

Jose told me he will not approve this or withdraw his objection now that everything he complained about was either his mistake reviewing or resolved. He said he was "commenting" and not "reviewing". Therefore, if it's okay, I don't want his name on the reviewed by trailer for this commit. We have a precedent of doing this already.

@concussious
Copy link
Contributor Author

concussious commented Nov 15, 2024 •
edited
Loading

From a sincere place of caring, and with respect and reverence, I would like to humbly ask the masters to consider that this bikeshed is, in fact, 15% better than what I originally proposed. However, I had a watertight, painted bikeshed ready before the code slush, and now new users will be piling their bikes in the rain until June 2025, and complaining to me about it, and that hurts my feelings.

You guys taught me everything, I am not going anywhere, but I am hurt. This negatively impacts me as the Manual Pages Mentor on the community discord. This negatively effects the perception of BSD doc culture when I am busy working, and new users ask how manual page search works (because emulates $unixtool is not an explanation), and someone else answers incorrectly before I have time to take care of it.

This is about showing new users that structured, carefully considered information written by competent passionate people is more accessible than asking chatgpt to hallucinate disinformation at the cost of 2.9 WH per query.

@jlduran
Copy link
Member

jlduran commented Nov 15, 2024

@concussious commented:
Jose told me he will not approve this or withdraw his objection now that everything he complained about was either his mistake reviewing or resolved. He said he was "commenting" and not "reviewing". Therefore, if it's okay, I don't want his name on the reviewed by trailer for this commit. We have a precedent of doing this already.

For completeness, I agree with the proposed changes, the way they are at this point in time, therefore "withdrawing" my objections.
However, the rest of the statement above still holds true.

@concussious
man.1: Improve search + spdx
61b3771 
People are often stunned by robust manual search functionality on
the community discord, so improve introductory doc regarding search by:

+ explain what search related flags do instead of using similies
+ consolidate and standardize search options in synopsis and usage
+ mention that a page or file can be specified in synopsis and example
+ call regular expressions `expression`, which searches to re_format(7)
+ crossreference the regular expression manual instead of egrep(1)
+ improve MANPATH xref flow and explanation, matching MAILPATH in sh(1)
+ mark up aditional semantics for their inclusion in apropos

While here:
+ use consistent spacing and form (Ql) for quoted literals
+ clean a few linter errors regarding self xref and nospace
+ reset a list width to indent for consistency with style.mdoc
+ tidy examples + align files + tag spdx

Outstanding:
- example 3 shows no results on a typical bsdinstall'd system

MFC after:	3 days
Reviewed by:	elliejs, emaste, Jessica Hawkwell, jlduran
bsdimp
bsdimp approved these changes Jan 24, 2025
View reviewed changes
Copy link
Member

@bsdimp bsdimp left a comment

Choose a reason for hiding this comment

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

Now that you have a commit bit, you can land this yourself, no?

Approved-by: imp

since it pre-dated things. Though if your mentor wants everything to go through them, we can do that too.

@concussious
Copy link
Contributor Author

Yes! Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jlduran jlduran jlduran left review comments

@LadySerenaKitty LadySerenaKitty LadySerenaKitty left review comments

@emaste emaste emaste left review comments

@bsdimp bsdimp bsdimp approved these changes

@elliejs elliejs elliejs approved these changes

Assignees

@bsdimp bsdimp

Labels
changes-required
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@concussious @bsdimp @jlduran @LadySerenaKitty @emaste @elliejs