bbatsov / emacs-lisp-style-guide

A community-driven Emacs Lisp style guide
1.08k stars 53 forks source link

Use descriptive keyword argument instead of `t` when boolean flag is expected #30

Open Fuco1 opened 9 years ago

Fuco1 commented 9 years ago

For example, functions which expects 5 arguments, all of which are t can get very confusing with calls like (foo t nil t nil t). It is not obvious what to do.

Recently I've been naming such arguments with descriptive names and then pass :argument-name keyword as a value. For example

(defun update (&optional force)
   "Update the list of values. If FORCE is non-nil, invalidate cache first.")

You would then call it (update) or (update :force) instead of (update t).

I think there is very little confusion possible over keyword-argument-functions and keyword-as-flags usage, especially when you can consult the documentation in about 1 second.

What do you think people?

Malabarba commented 9 years ago

I really like doing this too. It makes code much easier to understand when you don't have to visit every function's documentation to check what all those ts stand for.

That said, it's (sadly) not a practice I see used very often (do correct me if I'm wrong). Should the guide recommend something not widely used?

swsnr commented 9 years ago

@Fuco1 @Bruce-Connor I'd use (update 'force), though. It's not any better than :force, but avoids confusion with keywords, and—more importantly—it's the style that seems to be used throughout Emacs itself, and imho we should follow that in this style guide.

Malabarba commented 9 years ago

Agreed

bbatsov commented 9 years ago

I like @lunaryorn's suggestion.

bbatsov commented 9 years ago

A somewhat related rule would be to avoid naming prefix arguments ARG or PREFIX.

Fuco1 commented 9 years ago

And how else would you call the prefix argument? I don't know, prefix argument is rarely supplied from lisp (non-interactively), and when, 99% of the time it means either "repeat this thing this many times" or "toggle if non-nil". Calling it a repeat or toggle is not adding much.

swsnr commented 9 years ago

@Fuco1 Well, I do think that these names indeed add something: They convey what the prefix argument is used for, i.e. whether as toggle or boolean flag or for repetition, and that in turn helps users of the function, because the signature is now clear: I wouldn't need to read the docstring now in order to find out whether the prefix argument toggles or repeats.

Imho, as a general rule, you should always prefer a specific name about a generic one, even if the context of the generic name (the use as prefix argument) lets the user generally infer the meaning of the generic name. It's just easier for users.

Malabarba commented 9 years ago

Using TOGGLE as the example.

So a specific name is more useful overall. You can also change the function's calling convention so that the argument is displayed as TOGGLE-PREFIX in the documentation (which is maximally descriptive), without changing its name in the actual code, but that seems like overkill.

bbatsov commented 9 years ago

Ping :-)

Fuco1 commented 8 years ago

One advantage of :foo over 'foo is that they are fontified differently. For about a year now I use a special rule to fontify constants, such as 'foo so that helps (for me personally).

I don't mind either way, :arg or 'arg is fine, I would still slightly prefer :arg.

Fuco1 commented 6 years ago

@bbatsov Is this project still alive? Can you assign this to me, I'll add a note somewhere about this. Let's go with the 'force style, it is used in Emacs itself which is a good point and worth following.

bbatsov commented 6 years ago

Yeah, it is. And every month I plan to come back to it and write some many things, but I'm buried with work, life and side projects. I've made you a collaborator, so feel free to make things happen! :-)

Fuco1 commented 6 years ago

@bbatsov I have a secret project writing an elisp static analyser kind of thing... so I would eventually like to include these rules as a plugin there. But I'm also not making a lot of progress (https://github.com/Fuco1/Elsa)

bbatsov commented 6 years ago

Nice!

And I know that @gonewest818 has been working on reviving elint recently. I'd be happy to help as well, but that's unlikely to happen any time soon.

gonewest818 commented 6 years ago

@Fuco1: fwiw, I adopted elisp-lint from the original author (who has since moved onto other things) and did a little spring cleaning on it a few months back. You'll see it's fairly limited in what it does -- either doing string matches on the text file or, in some cases, invoking existing emacs "infrastructure" packages like bytecomp, checkdoc, check-declare, or package to validate the code. There's definitely room for more/better tools in this area. If you see anything that can/ought to be changed in elisp-lint please let me know.

Fuco1 commented 6 years ago

@gonewest818 I have a completely different approach. What I am doing is reading the forms, parsing the structure and actually running type inference to see what is going on. You can annotate the forms where types are ambiguous to help the engine to do a better job. You get a lot of information like the scope, variables, globals, functions and all sorts of other things at each point in the AST so the decisions are much more informed.

So this will be 100% syntactically based which allows for massively broader class of checks. The linting stuff would only piggyback on the existing infrastructure but that's not the main goal.

gonewest818 commented 6 years ago

@Fuco1 yep, got it.