Skip to content

Example of doc changes to be within 80 char limit#203

Closed
matsl wants to merge 2 commits intomasterfrom
long_doc_strings
Closed

Example of doc changes to be within 80 char limit#203
matsl wants to merge 2 commits intomasterfrom
long_doc_strings

Conversation

@matsl
Copy link
Copy Markdown
Collaborator

@matsl matsl commented Jul 9, 2022

What

Example of doc changes to be within 80 char limit.

Why

Looking at how it would be to go down the route to fix the 80 char limit. You suggested that I would do a couple of docs to see what it means. I did slightly more but before doing all I would want to share it for feedback. It is really a lot to do but it triggers some good discussion points, see below.

Some things I noticed while doing this work:

  • elisp mode defaults to fill column of 65 chars which is a bit surprising. We should probably have a style guide so that our doc strings look similar. Tried to look for a general style guide for Emacs about that but did not find it but maybe there is. I have for this work kept the default when formatting the comment body. I have tried to do that in as few cases as possible as that creates a bigger diff.
  • Some first line comments are really close to 80 chars so simple things like dropping a the or similar suffice to fix those.
  • Some first line comments mentions Hyperbole, or other facts that should be clear for the context, and can be fixed by removing these.
  • Some first line comments tries to capture everything about a function. Both the major functionality but also all alternatives. This might be the ones that would be hardest to change, but to be honest, I find them sometimes hard to read and understand. So forcing us to describe it within 80 chars would have the good side effect of making them more readable since long sentences are harder to read.
  • Some functions have slightly misleading names. Example of that are the -p (predicate functions) where the comment line has to compensate for that by explain more what the function does. A good function name could make the task of the first comment line smaller and by that possible to capture in the less space.
  • Functions with a lot of alternatives, controlled by optional arguments, might benefit from being split into multiple functions making each function both easier to test and to describe?
  • Some techniques I have used:
    • Simply write a shorter first comment line and kept the original comment line in the comment body.
    • Cut the comment line in two. Moving the second part of it into the comment body.
    • Cut our part of the doc in the first line and then write a longer part describing the cut out piece in the comment body.
  • My flycheck warned about single quotes in some doc strings so I fixed those as well. Not sure that is necessary. It just happened when I was at it. So feel free to ignore those changes.

@matsl matsl requested a review from rswgnu July 9, 2022 08:52
@rswgnu
Copy link
Copy Markdown
Owner

rswgnu commented Jul 16, 2022

See comments above so far to integrate. I have more to review.

@matsl
Copy link
Copy Markdown
Collaborator Author

matsl commented Jul 17, 2022

Hi @rswgnu, I can't see any comments I'm afraid. Could it be that you have started a review and that is it not completed yet? I normally don't use the review feature but write single comments so I'm not sure how the review thing works. I think you need to finish it before the comments becomes available to me! (or something like that. I really should learn how that can be used. Might be a good thing.)

@rswgnu
Copy link
Copy Markdown
Owner

rswgnu commented Jul 17, 2022 via email

@matsl
Copy link
Copy Markdown
Collaborator Author

matsl commented Jul 18, 2022

@rswgnu So your suggestion is that we shall close this PR and that I should come back with smaller PRs?

@matsl matsl force-pushed the long_doc_strings branch from dae9073 to 1b5cdd3 Compare July 18, 2022 20:13
rswgnu
rswgnu requested changes Jul 18, 2022
View reviewed changes
Copy link
Copy Markdown
Owner

@rswgnu rswgnu left a comment

Choose a reason for hiding this comment

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

This has the comments I made with updates I made but just for the first several entries. I do suggest you make these changes, close this PR, and then submit smaller, chunked ones that I can process each day for you. Thanks, Bob

hact.el Outdated

(defun symtable:operate (operation symbol-or-name symtable)
"Call hash-table function OPERATION with Hyperbole SYMBOL-OR-NAME as key upon SYMTABLE.
"Call hash-table function OPERATION with SYMBOL-OR-NAME as key upon SYMTABLE.
Copy link
Copy Markdown
Owner

@rswgnu rswgnu Jul 10, 2022

Choose a reason for hiding this comment

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

Try this style and put 'Hyperbole' back when referring to symtables:
Call hash-table OPERATION with Hyperbole SYMBOL-OR-NAME key for SYMTABLE.

hact.el Outdated

(defsubst symtable:actype-p (symbol-or-name)
"Return the Elisp symbol given by SYMBOL-OR-NAME if it is a Hyperbole action type name, else nil."
"Return the symbol given by SYMBOL-OR-NAME if it is a action type."
Copy link
Copy Markdown
Owner

@rswgnu rswgnu Jul 10, 2022

Choose a reason for hiding this comment

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

Return SYMBOL-OR-NAME if it is a Hyperbole action type, else nil.

hact.el Outdated

(defsubst symtable:ibtype-p (symbol-or-name)
"Return the Elisp symbol given by SYMBOL-OR-NAME if it is a Hyperbole implicit button type name, else nil."
"Return the symbol given by SYMBOL-OR-NAME if it is an implicit button type."
Copy link
Copy Markdown
Owner

@rswgnu rswgnu Jul 10, 2022

Choose a reason for hiding this comment

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

Return SYMBOL-OR-NAME if it is a Hyperbole implicit button type, else nil.

hact.el Outdated

(defun symtable:get (symbol-or-name symtable)
"Return the Elisp symbol given by Hyperbole SYMBOL-OR-NAME if it is in existing SYMTABLE, else nil.
"Return the symbol given by SYMBOL-OR-NAME if it is in existing SYMTABLE.
Copy link
Copy Markdown
Owner

@rswgnu rswgnu Jul 10, 2022

Choose a reason for hiding this comment

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

Return SYMBOL-OR-NAME if it is a Hyperbole symbol table, else nil.

hact.el

(defun symtable:remove (symbol-or-name symtable)
"Remove the Elisp symbol given by Hyperbole SYMBOL-OR-NAME if it is in existing SYMTABLE.
"Remove the symbol given by SYMBOL-OR-NAME if it is in existing SYMTABLE.
Copy link
Copy Markdown
Owner

@rswgnu rswgnu Jul 10, 2022

Choose a reason for hiding this comment

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

Remove SYMBOL-OR-NAME if it is in existing SYMTABLE.

hargs.el Outdated
list-positions-flag exclude-regexp)
"Return a normalized, single line, delimited string that point is within the first line of, or nil.
"Return a delimited string that point is within the first line of, or nil.
The string is normalized, single line.
Copy link
Copy Markdown
Owner

@rswgnu rswgnu Jul 10, 2022

Choose a reason for hiding this comment

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

The string is normalized and reduced to a single line.

hib-debbugs.el Outdated

(defun debbugs-gnu-query:help (_but)
"Make a GNU debbugs id number at point (optionally prefixed with a # sign) display the pretty pretted status of the bug id.
"Make a GNU display the pretty pretted status of the bug id.
Copy link
Copy Markdown
Owner

@rswgnu rswgnu Jul 10, 2022

Choose a reason for hiding this comment

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

Make a Gnu debbugs id number at point display the pretty-printed bug status.

hib-debbugs.el Outdated

(defun debbugs-gnu-mode:help (&optional _but)
"Make a Gnu debbugs listing entry at point pretty print the status of the issue to a window below."
"Pretty print the status of a Debbugs listing entry to a window below."
Copy link
Copy Markdown
Owner

@rswgnu rswgnu Jul 10, 2022

Choose a reason for hiding this comment

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

Make a Gnu debbugs listing at point pretty-print its status to a window below.

hib-debbugs.el Outdated
may also be included at the front of the string to limit the query to a particular
issue number. Note that `issue' or `debbugs' may be used as well in place of `bug'."
"Display the results of a Gnu debbugs query.
Parse and apply attributes from URL-QUERY-STRING.
Copy link
Copy Markdown
Owner

@rswgnu rswgnu Jul 10, 2022

Choose a reason for hiding this comment

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

Show the results of a Gnu debbugs query with attributes from URL-QUERY-STRING.

(Replace first 2 lines with this single line).

hib-debbugs.el Outdated
Attribute may be a symbol or a string. Common attributes include: status,
severity, and package."
"Display the results of a Gnu debbugs query.
Apply attributes from QUERY-ATTRIBUTE-LIST to. Each element of
Copy link
Copy Markdown
Owner

@rswgnu rswgnu Jul 10, 2022

Choose a reason for hiding this comment

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

Show the results of a Gnu debbugs query with attributes from QUERY-ATTRIBUTE-LIST.
Each element of...

@matsl
Copy link
Copy Markdown
Collaborator Author

matsl commented Jul 20, 2022

Closing this branch for providing smaller PRs.

@matsl matsl closed this Jul 20, 2022
@matsl matsl mentioned this pull request Jul 20, 2022
Merged
@rswgnu
Copy link
Copy Markdown
Owner

rswgnu commented Oct 11, 2022 via email

@rswgnu
Copy link
Copy Markdown
Owner

rswgnu commented Oct 11, 2022 via email

@matsl matsl deleted the long_doc_strings branch April 8, 2023 08:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rswgnu rswgnu rswgnu requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@matsl @rswgnu