all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* bug#6598: Documentation: progmodes/grep.el -- numerous errors ommissions and opportunities for improvement.
@ 2010-07-10  0:07 MON KEY
  2019-10-11 20:10 ` Lars Ingebrigtsen
  0 siblings, 1 reply; 4+ messages in thread
From: MON KEY @ 2010-07-10  0:07 UTC (permalink / raw)
  To: 6598

Documentation progmodes/grep.el -- numerous errors ommissions and
opportunities for improvement.

--
There are 13 docstring references of the form: `grep-compute-defaults'
in progmodes/grep.el and 7 more in lisp/loaddefs.el including the
grep-compute-defaults autoload yet the function itself is not
documented.


-- 
The following functions accept a CONFIRM argument:
`lgrep', `rgrep', `zrgrep'

There is no explicit documentation of the CONFIRM parameter in any of
the docstrings for these functions.

Likewise, `grep', `lgrep', `rgrep', `grep-find' docs have the
following two vacuous statements:

 "[Cc]ollect output in a buffer."
  "in the grep output buffer"

but do not specify which buffer e.g. the default: "*grep*".

--
The constant `grep-expand-keywords' has the typo:

 "If car of an element matches, the cdr is evalled in to get the"
                                                     ^
Should probably be: "in order to"

Also, the doc makes mention that one should
 "Note dynamic scoping of variables."

but doesn't discuss what these variables are, nor why they should be
noted.

--
The function `grep-expand-template' uses the term "Patch":

 "Patch grep COMMAND string replacing <C>, <D>, <F>, <R>, and <X>."

This terminology should be avoided b/c it conflates with diff's `patch'.
Likewise, it isn't clear what is getting replaced by the "patch".

--
The function `grep-read-files' does not document the REGEXP parameter.

--
The variable `grep-find-ignored-directories' should indicate that it
defaults to the value of the variable `vc-directory-exclusion-list' as
this is an option that is user customizable it may be useful to simply
modify the value of that variable instead.

--
The variables `grep-template', `grep-find-template',
`grep-highlight-matches', `grep-find-command', `grep-use-null-device'
say:

 " {...} to change the default value, use Customize or call the function
   `grep-apply-setting'."

"Customize" should be: "\\[customize]"

Likewise, where this is a customizable variable and the intent seems
to be that the user _customize_ them the sentence:

 "You can customize this variable."

is already present at the bottom of each of these function's
docstrings.

-- 
The variables `grep-template', `grep-find-template'
make mention of the `place holders":

 <C>, <F>, <X>, <R>, <N>

saying:

 "The following place holders should be present in the string:"

But do not describe _how_ they should be present.
It is not at all clear how these will affect the commands they inform.
An example of their usage would be exceedingly beneficial.

--
The manual (info "(emacs)Dired and Find") says:

 "Remember to write the regular expression for `grep', not for Emacs"

Its fine if one remembers to do this, but what if it isn't known how
to do this in the first place.

 "Management to Bobby:
  Okay Bobby, today is your first day at the nuclear power plant...
  We're sure you'll do fine at your new station.
  BTW don't forget to secure the Slotin shims before leaving for lunch.
  You wouldn't wanna end up like poor Louis.

  Bobby to management:
  Wait!!! WTF is a Slotin shim... who is Louis?

  Management to Bobby:
  Don't worry you'll be fine...

  Bobby to self (a few hours after returning from lunch - shims forgotten):
  Must've ate something funny, I think I'm gonna puke."

--
/s_P\





^ permalink raw reply	[flat|nested] 4+ messages in thread

* bug#6598: Documentation: progmodes/grep.el -- numerous errors ommissions and opportunities for improvement.
  2010-07-10  0:07 bug#6598: Documentation: progmodes/grep.el -- numerous errors ommissions and opportunities for improvement MON KEY
@ 2019-10-11 20:10 ` Lars Ingebrigtsen
  2019-10-12  6:52   ` Eli Zaretskii
  0 siblings, 1 reply; 4+ messages in thread
From: Lars Ingebrigtsen @ 2019-10-11 20:10 UTC (permalink / raw)
  To: MON KEY; +Cc: 6598

MON KEY <monkey@sandpframing.com> writes:

> Documentation progmodes/grep.el -- numerous errors ommissions and
> opportunities for improvement.
>
> --
> There are 13 docstring references of the form: `grep-compute-defaults'
> in progmodes/grep.el and 7 more in lisp/loaddefs.el including the
> grep-compute-defaults autoload yet the function itself is not
> documented.

OK, added.

> -- 
> The following functions accept a CONFIRM argument:
> `lgrep', `rgrep', `zrgrep'
>
> There is no explicit documentation of the CONFIRM parameter in any of
> the docstrings for these functions.

Ditto.

> Likewise, `grep', `lgrep', `rgrep', `grep-find' docs have the
> following two vacuous statements:
>
>  "[Cc]ollect output in a buffer."
>   "in the grep output buffer"
>
> but do not specify which buffer e.g. the default: "*grep*".

Ditto.

> The constant `grep-expand-keywords' has the typo:
>
>  "If car of an element matches, the cdr is evalled in to get the"
>                                                      ^
> Should probably be: "in order to"
>
> Also, the doc makes mention that one should
>  "Note dynamic scoping of variables."
>
> but doesn't discuss what these variables are, nor why they should be
> noted.

I've now rephrased it and documented the variables.

> --
> The function `grep-expand-template' uses the term "Patch":
>
>  "Patch grep COMMAND string replacing <C>, <D>, <F>, <R>, and <X>."
>
> This terminology should be avoided b/c it conflates with diff's `patch'.
> Likewise, it isn't clear what is getting replaced by the "patch".

I used "expand" instead.

> --
> The function `grep-read-files' does not document the REGEXP parameter.

Fixed.

> The variable `grep-find-ignored-directories' should indicate that it
> defaults to the value of the variable `vc-directory-exclusion-list' as
> this is an option that is user customizable it may be useful to simply
> modify the value of that variable instead.

Fixed.

> The variables `grep-template', `grep-find-template',
> `grep-highlight-matches', `grep-find-command', `grep-use-null-device'
> say:
>
>  " {...} to change the default value, use Customize or call the function
>    `grep-apply-setting'."
>
> "Customize" should be: "\\[customize]"

Fixed.

> Likewise, where this is a customizable variable and the intent seems
> to be that the user _customize_ them the sentence:
>
>  "You can customize this variable."
>
> is already present at the bottom of each of these function's
> docstrings.

I'm guessing the intent here is to say that you can use
`grep-apply-setting' instead of Customize, so I think the repetition is OK.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





^ permalink raw reply	[flat|nested] 4+ messages in thread

* bug#6598: Documentation: progmodes/grep.el -- numerous errors ommissions and opportunities for improvement.
  2019-10-11 20:10 ` Lars Ingebrigtsen
@ 2019-10-12  6:52   ` Eli Zaretskii
  2019-10-12 17:56     ` Lars Ingebrigtsen
  0 siblings, 1 reply; 4+ messages in thread
From: Eli Zaretskii @ 2019-10-12  6:52 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 6598, monkey

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Date: Fri, 11 Oct 2019 22:10:57 +0200
> Cc: 6598@debbugs.gnu.org
> 
> > The following functions accept a CONFIRM argument:
> > `lgrep', `rgrep', `zrgrep'
> >
> > There is no explicit documentation of the CONFIRM parameter in any of
> > the docstrings for these functions.
> 
> Ditto.

Thanks, but this:

> +If CONFIRM, the user will be given an opportunity to edit the

is not our style.  Please say "If CONFIRM is non-nil, the user..."
instead.

(I fixed those in grep.el.)





^ permalink raw reply	[flat|nested] 4+ messages in thread

* bug#6598: Documentation: progmodes/grep.el -- numerous errors ommissions and opportunities for improvement.
  2019-10-12  6:52   ` Eli Zaretskii
@ 2019-10-12 17:56     ` Lars Ingebrigtsen
  0 siblings, 0 replies; 4+ messages in thread
From: Lars Ingebrigtsen @ 2019-10-12 17:56 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 6598, monkey

Eli Zaretskii <eliz@gnu.org> writes:

>> +If CONFIRM, the user will be given an opportunity to edit the
>
> is not our style.  Please say "If CONFIRM is non-nil, the user..."
> instead.
>
> (I fixed those in grep.el.)

Thanks.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2019-10-12 17:56 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-07-10  0:07 bug#6598: Documentation: progmodes/grep.el -- numerous errors ommissions and opportunities for improvement MON KEY
2019-10-11 20:10 ` Lars Ingebrigtsen
2019-10-12  6:52   ` Eli Zaretskii
2019-10-12 17:56     ` Lars Ingebrigtsen

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.