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

Docstrings and argument names #9

Open
alphapapa opened this issue Jun 10, 2022 · 0 comments
Open

Docstrings and argument names #9

alphapapa opened this issue Jun 10, 2022 · 0 comments

Comments

@alphapapa
Copy link

alphapapa commented Jun 10, 2022

Hi,

Thanks for your work on this library. It looks very useful.

A quick suggestion: many of the functions' docstrings are written like, "Get foo," but it would be more conventional (and clearer, IMHO) to write them in terms of the values they return (because saying that a function "gets" something doesn't necessarily say what it does with it, or what value is returned).

For example, take this function:

(defun ct-format-argb (C &optional opacity end)
  "Argb formatting:
Pass in C and OPACITY 0-100, get a string representation of C
as follows: '#AAFFFFFF', where AA is a hex pair for the alpha,
followed by FF times 3 hex pairs for red, green, blue. If END is
truthy, then format will be '#FFFFFFAA'."

Instead, I would recommend something like:

(defun ct-format-argb (color &optional opacity end)
  "Return COLOR as a hex RGB string.
OPACITY may be an integer from 0-100.  If END is non-nil, 
append the alpha digits to the end of the color; otherwise, 
prepend them to the beginning."

That conforms to the published Elisp conventions, which makes the code much easier to understand for those who may read and contribute to it.

You'll notice that I also renamed the primary argument from C to color. In Lisp, abbreviating symbol names is usually unnecessary and unconventional. As well, using whole words makes code clearer and easier to read.

The checkdoc command that's built-in to Emacs can help point out issues like these. You may also find my makem.sh script helpful for running these kinds of checks in a clean Emacs configuration, e.g. with M-x compile.

@neeasade neeasade mentioned this issue Oct 25, 2024
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant