Attached is a revised patch addressing comments of Eli and Stefan. Further clarifications below:- Stefan Monnier writes: >> (defun prepare-abbrev-list-buffer (&optional local) >> + "Return *Abbrevs* buffer for the caller to select or display. > > A few points: > > - I don't think the docstring should document to the name of the buffer. > - Docstrings should usually say what the function does rather than what > the callers are expected to do with the result. Initially, I had simply lifted the text from the file ChangeLog.1 1985-12-13 Richard M. Stallman (rms@prep) * abbrev.el (prepare-abbrev-list-buffer, list-abbrevs, edit-abbrevs): Some cleanups. prepare-... now does all the work and returns the buffer for the caller to select or display. But I have now rewritten. > - To me, the above description makes it sound like the function does > little more than `(get-buffer "*Abbrevs*")`. > IOW, it should say that it fills the buffer with "some" representation > of "some" abbrevs. > >> +LOCAL is a flag, if non-nil display only local abbrevs. >> +" > > We don't like our docstrings to end with a newline. Fixed. > >> +A negative ARG means to undefine the specified abbrev. > [...] >> +This command reads the abbreviation from the minibuffer. > > We should say this before referring to it via "the specified abbrev". Fixed in both add-abbrev and inverse-add-abbrev. I initially adapted the docstring from surrounding callers like add-global-abbrev, but have now rewritten. > >> +TYPE of abbrev includes \"Global\", \"Mode\". > > Here you made the same mistake of documenting what the callers do rather > than what the function does. Fixed, rewritten, it turns out the ARG string is purely descriptive. > >> +See `add-global-abbrev', `add-mode-abbrev' for caller examples. > > We usually don't include this in docstrings. > We have `xref` and `grep` for those kinds of things. Fixed >> (defun abbrev--describe (sym) >> + "Describe abbrev SYM. >> +Used to generate a table by `insert-abbrev-table-description'." > > Similarly here I wouldn't mention the caller. Instead I'd try and > explain what kind of description is generated and where it's placed > (at point in the current buffer? As a return values? ...). Fixed with this detail after examining the code. > >> (defun abbrev--possibly-save (query &optional arg) >> + "Hook function for use by `save-some-buffer-functions'. >> +Associated meaning for QUERY and ARG." > > Hook functions are one of the exceptions where it's often OK to refer to > how it's used in the docstring, like you do here (e.g. so we don't have > to repeat what `query` and `arg` are for). > But it should also say what it actually does. Fixed. > > > Stefan