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

finger: add page #2141

Merged
merged 5 commits into from
Jun 19, 2018
Merged

finger: add page #2141

merged 5 commits into from
Jun 19, 2018

Conversation

MasterOdin
Copy link
Collaborator


  • The page (if new), does not already exist in the repo.

  • The page (if new), has been added to the correct platform folder:
    common/ if it's common to all platforms, linux/ if it's Linux-specific, and so on.

  • The page has 8 or fewer examples.

  • The PR is appropriately titled:
    <command name>: add page for new pages, or <command name>: <description of changes> for pages being edited.

  • The page follows the contributing guidelines.

Copy link
Member

@agnivade agnivade left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. Please look at my comments below.


> User information lookup program.

- Display the user's login name, real name, terminal name and write status (as a "\*" after the terminal name if write permission is denied), idle time, login time, office location and office phone number:
Copy link
Member

Choose a reason for hiding this comment

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

We should start with the simplest invocation - finger. And probably shorten the description to Display the user's info


`finger -s`

- Produce a multi-line format displaying all the information described for the option ```-s``` as well as the user's home directory, home phone number, login shell, mail status, and the contents of the files ".plan", ".project", ".pgpkey" and ".forward" from the user's home directory:
Copy link
Member

Choose a reason for hiding this comment

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

This is copied verbatim from the man page. Please shorten it to a simpler version, and if possible use a word starting with l.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Shortened it up, but no word came to mind that starts with an l.


`finger -l`

- Prevent the ```-l``` option of finger from displaying the contents of the ".plan", ".project" and ".pgpkey" files:
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this option is required to be in a tldr page.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed.

@tldr-bot
Copy link

The build for this PR has failed with the following error(s):

pages/common/finger.md:5: TLDR104 Example descriptions should prefer infinitive tense (e.g. write) over present (e.g. writes) or gerund (e.g. writing)
pages/common/finger.md:17: TLDR104 Example descriptions should prefer infinitive tense (e.g. write) over present (e.g. writes) or gerund (e.g. writing)

Please fix the error(s) and push again.

@owenvoke owenvoke added the new command Issues requesting creation of a new page. label Jun 12, 2018
Copy link
Member

@owenvoke owenvoke left a comment

Choose a reason for hiding this comment

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

I think that the -s and -l descriptions are still pretty long for a tldr page. 🤔 I can't think of a good way to shorten them though.


`finger -s`

- Produce multi-line format displaying same information as `-s` as well as user's home directory, home phone number, login shell, mail status as well as contents of files ".plan", ".project", ".pgpkey", and ".foward" in their home directory:
Copy link
Member

Choose a reason for hiding this comment

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

This part can be removed as well as contents of files ".plan", ".project", ".pgpkey", and ".foward" in their home directory and just put etc. in place of it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.


`finger {{username}}`

- Display the user's login name, real name, terminal name and write status (as a "\*" after the terminal name if write permission is denied), idle time, login time, office location and office phone number:
Copy link
Member

Choose a reason for hiding this comment

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

Remove - write status (as a "\*" after the terminal name if write permission is denied), idle time, login time, office location and office phone number and just put other information.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.


`finger -s`

- Produce multi-line format displaying same information as `-s` as well as user's home directory, home phone number, login shell, mail status as well as contents of files ".plan", ".project", ".pgpkey", and ".foward" in their home directory:
Copy link
Member

Choose a reason for hiding this comment

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

Produce a multi-line output format

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

differs from the man page description, but I agree that it's clearer this way.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, the whole point is to differ from the man page. 😄

Copy link
Member

@owenvoke owenvoke left a comment

Choose a reason for hiding this comment

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

Looks fine to me I think.

@agnivade agnivade merged commit 4cf731c into tldr-pages:master Jun 19, 2018
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants