[PATCH] emacs: remove hardcoded defaults values from docstrings

[PATCH] emacs: remove hardcoded defaults values from docstrings

[david]

From: David Bremner <bremner@debian.org>

These functions refer to default values of variables, but it seems
less confusing and less likely to get out of date to just allow the
user to follow the help cross-reference links.
---
 emacs/notmuch-show.el | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

another proposed change from our notmuch-pick discussions.

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 613e666..600e802 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -1835,10 +1835,9 @@ search results instead."
   "Archive each message in thread.
 
 Archive each message currently shown by applying the tag changes
-in `notmuch-archive-tags' to each (remove the \"inbox\" tag by
-default). If a prefix argument is given, the messages will be
-\"unarchived\", i.e. the tag changes in `notmuch-archive-tags'
-will be reversed.
+in `notmuch-archive-tags' to each. If a prefix argument is given,
+the messages will be \"unarchived\", i.e. the tag changes in
+`notmuch-archive-tags' will be reversed.
 
 Note: This command is safe from any race condition of new messages
 being delivered to the same thread. It does not archive the
@@ -1865,10 +1864,9 @@ buffer."
   "Archive the current message.
 
 Archive the current message by applying the tag changes in
-`notmuch-archive-tags' to it (remove the \"inbox\" tag by
-default). If a prefix argument is given, the message will be
-\"unarchived\", i.e. the tag changes in `notmuch-archive-tags'
-will be reversed."
+`notmuch-archive-tags' to it. If a prefix argument is given, the
+message will be \"unarchived\", i.e. the tag changes in
+`notmuch-archive-tags' will be reversed."
   (interactive "P")
   (when notmuch-archive-tags
     (apply 'notmuch-show-tag-message
-- 
1.8.2.rc2

[PATCH] emacs: add `notmuch-archive-tags' cross references in docstrings

[david]

From: David Bremner <bremner@debian.org>

Several function docstrings refer to behaviour in docstrings that is
really controlled by notmuch-archive-tags. Add cross references, and
replace hardcoding.
---
 emacs/notmuch-show.el |  4 ++--
 emacs/notmuch.el      | 27 ++++++++++++++++-----------
 2 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 600e802..c281f05 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -1529,8 +1529,8 @@ This command is intended to be one of the simplest ways to
 process a thread of email. It works exactly like
 notmuch-show-advance, in that it scrolls through messages in a
 show buffer, except that when it gets to the end of the buffer it
-archives the entire current thread, (remove the \"inbox\" tag
-from each message), kills the buffer, and displays the next
+archives the entire current thread, (apply changes in
+`notmuch-archive-tags'), kills the buffer, and displays the next
 thread from the search from which this thread was originally
 shown."
   (interactive)
diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 5a8c957..e78334d 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -386,17 +386,22 @@ number of matched messages and total messages in the thread,
 participants in the thread, a representative subject line, and
 any tags).
 
-Pressing \\[notmuch-search-show-thread] on any line displays that thread. The '\\[notmuch-search-add-tag]' and '\\[notmuch-search-remove-tag]'
-keys can be used to add or remove tags from a thread. The '\\[notmuch-search-archive-thread]' key
-is a convenience for archiving a thread (removing the \"inbox\"
-tag). The '\\[notmuch-search-tag-all]' key can be used to add and/or remove tags from all
-messages (as opposed to threads) that match the current query.  Use with caution, as this
-will also tag matching messages that arrived *after* constructing the buffer.
-
-Other useful commands are '\\[notmuch-search-filter]' for filtering the current search
-based on an additional query string, '\\[notmuch-search-filter-by-tag]' for filtering to include
-only messages with a given tag, and '\\[notmuch-search]' to execute a new, global
-search.
+Pressing \\[notmuch-search-show-thread] on any line displays that
+thread. The '\\[notmuch-search-add-tag]' and
+'\\[notmuch-search-remove-tag]' keys can be used to add or remove
+tags from a thread. The '\\[notmuch-search-archive-thread]' key
+is a convenience for archiving a thread (applying changes in
+`notmuch-archive-tags'). The '\\[notmuch-search-tag-all]' key can
+be used to add and/or remove tags from all messages (as opposed
+to threads) that match the current query.  Use with caution, as
+this will also tag matching messages that arrived *after*
+constructing the buffer.
+
+Other useful commands are '\\[notmuch-search-filter]' for
+filtering the current search based on an additional query string,
+'\\[notmuch-search-filter-by-tag]' for filtering to include only
+messages with a given tag, and '\\[notmuch-search]' to execute a
+new, global search.
 
 Complete list of currently available key bindings:
 
-- 
1.8.2.rc2

Re: [PATCH] emacs: remove hardcoded defaults values from docstrings

[Tomi Ollila]

On Sun, Jun 02 2013, david@tethera.net wrote:

> From: David Bremner <bremner@debian.org>
>
> These functions refer to default values of variables, but it seems
> less confusing and less likely to get out of date to just allow the
> user to follow the help cross-reference links.
> ---

LGTM.

Tomi


>  emacs/notmuch-show.el | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
>
> another proposed change from our notmuch-pick discussions.
>
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index 613e666..600e802 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -1835,10 +1835,9 @@ search results instead."
>    "Archive each message in thread.
>  
>  Archive each message currently shown by applying the tag changes
> -in `notmuch-archive-tags' to each (remove the \"inbox\" tag by
> -default). If a prefix argument is given, the messages will be
> -\"unarchived\", i.e. the tag changes in `notmuch-archive-tags'
> -will be reversed.
> +in `notmuch-archive-tags' to each. If a prefix argument is given,
> +the messages will be \"unarchived\", i.e. the tag changes in
> +`notmuch-archive-tags' will be reversed.
>  
>  Note: This command is safe from any race condition of new messages
>  being delivered to the same thread. It does not archive the
> @@ -1865,10 +1864,9 @@ buffer."
>    "Archive the current message.
>  
>  Archive the current message by applying the tag changes in
> -`notmuch-archive-tags' to it (remove the \"inbox\" tag by
> -default). If a prefix argument is given, the message will be
> -\"unarchived\", i.e. the tag changes in `notmuch-archive-tags'
> -will be reversed."
> +`notmuch-archive-tags' to it. If a prefix argument is given, the
> +message will be \"unarchived\", i.e. the tag changes in
> +`notmuch-archive-tags' will be reversed."
>    (interactive "P")
>    (when notmuch-archive-tags
>      (apply 'notmuch-show-tag-message
> -- 
> 1.8.2.rc2
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH] emacs: remove hardcoded defaults values from docstrings

[Mark Walters]

both of these LGTM +1.

Note there are still some (an?) occurrences of this: eg
notmuch-search-archive-thread, and there is one in pick. But these might
as well go in anyway.

Best wishes

Mark

 On Sun, 02 Jun 2013, david@tethera.net wrote:
> From: David Bremner <bremner@debian.org>
>
> These functions refer to default values of variables, but it seems
> less confusing and less likely to get out of date to just allow the
> user to follow the help cross-reference links.
> ---
>  emacs/notmuch-show.el | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
>
> another proposed change from our notmuch-pick discussions.
>
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index 613e666..600e802 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -1835,10 +1835,9 @@ search results instead."
>    "Archive each message in thread.
>  
>  Archive each message currently shown by applying the tag changes
> -in `notmuch-archive-tags' to each (remove the \"inbox\" tag by
> -default). If a prefix argument is given, the messages will be
> -\"unarchived\", i.e. the tag changes in `notmuch-archive-tags'
> -will be reversed.
> +in `notmuch-archive-tags' to each. If a prefix argument is given,
> +the messages will be \"unarchived\", i.e. the tag changes in
> +`notmuch-archive-tags' will be reversed.
>  
>  Note: This command is safe from any race condition of new messages
>  being delivered to the same thread. It does not archive the
> @@ -1865,10 +1864,9 @@ buffer."
>    "Archive the current message.
>  
>  Archive the current message by applying the tag changes in
> -`notmuch-archive-tags' to it (remove the \"inbox\" tag by
> -default). If a prefix argument is given, the message will be
> -\"unarchived\", i.e. the tag changes in `notmuch-archive-tags'
> -will be reversed."
> +`notmuch-archive-tags' to it. If a prefix argument is given, the
> +message will be \"unarchived\", i.e. the tag changes in
> +`notmuch-archive-tags' will be reversed."
>    (interactive "P")
>    (when notmuch-archive-tags
>      (apply 'notmuch-show-tag-message
> -- 
> 1.8.2.rc2
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH] emacs: remove hardcoded defaults values from docstrings

[David Bremner]

Mark Walters <markwalters1009@gmail.com> writes:

> both of these LGTM +1.
>
> Note there are still some (an?) occurrences of this: eg
> notmuch-search-archive-thread, and there is one in pick. But these might
> as well go in anyway.

pushed both,

d

Re: [PATCH] emacs: remove hardcoded defaults values from docstrings

[Austin Clements]

I realize this already got pushed, but to me this seems like a
reversion for the 95% of users who haven't customized
notmuch-archive-tags and just want to know that archiving means
removing the inbox tag.  Why is this less confusing?  Users who have
customized notmuch-archive-tags will probably know that the "default"
doesn't apply to them.

Quoth david@tethera.net on Jun 02 at 11:04 am:
> From: David Bremner <bremner@debian.org>
> 
> These functions refer to default values of variables, but it seems
> less confusing and less likely to get out of date to just allow the
> user to follow the help cross-reference links.
> ---
>  emacs/notmuch-show.el | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
> 
> another proposed change from our notmuch-pick discussions.
> 
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index 613e666..600e802 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -1835,10 +1835,9 @@ search results instead."
>    "Archive each message in thread.
>  
>  Archive each message currently shown by applying the tag changes
> -in `notmuch-archive-tags' to each (remove the \"inbox\" tag by
> -default). If a prefix argument is given, the messages will be
> -\"unarchived\", i.e. the tag changes in `notmuch-archive-tags'
> -will be reversed.
> +in `notmuch-archive-tags' to each. If a prefix argument is given,
> +the messages will be \"unarchived\", i.e. the tag changes in
> +`notmuch-archive-tags' will be reversed.
>  
>  Note: This command is safe from any race condition of new messages
>  being delivered to the same thread. It does not archive the
> @@ -1865,10 +1864,9 @@ buffer."
>    "Archive the current message.
>  
>  Archive the current message by applying the tag changes in
> -`notmuch-archive-tags' to it (remove the \"inbox\" tag by
> -default). If a prefix argument is given, the message will be
> -\"unarchived\", i.e. the tag changes in `notmuch-archive-tags'
> -will be reversed."
> +`notmuch-archive-tags' to it. If a prefix argument is given, the
> +message will be \"unarchived\", i.e. the tag changes in
> +`notmuch-archive-tags' will be reversed."
>    (interactive "P")
>    (when notmuch-archive-tags
>      (apply 'notmuch-show-tag-message

Re: [PATCH] emacs: remove hardcoded defaults values from docstrings

[Tomi Ollila]

On Tue, Jun 04 2013, Austin Clements <amdragon@MIT.EDU> wrote:

> I realize this already got pushed, but to me this seems like a
> reversion for the 95% of users who haven't customized
> notmuch-archive-tags and just want to know that archiving means
> removing the inbox tag.  Why is this less confusing?  Users who have
> customized notmuch-archive-tags will probably know that the "default"
> doesn't apply to them.

Can we have both -- i.e. something like (default: "-inbox") in the 
docstring. When entering c-h f notmuch-show-archive-thread
The `notmuch-archive-tags' in text is a hyperlink to the documentation
of notmuch-archive-tags (which shows its current value)...


Tomi

>
> Quoth david@tethera.net on Jun 02 at 11:04 am:
>> From: David Bremner <bremner@debian.org>
>> 
>> These functions refer to default values of variables, but it seems
>> less confusing and less likely to get out of date to just allow the
>> user to follow the help cross-reference links.
>> ---
>>  emacs/notmuch-show.el | 14 ++++++--------
>>  1 file changed, 6 insertions(+), 8 deletions(-)
>> 
>> another proposed change from our notmuch-pick discussions.
>> 
>> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
>> index 613e666..600e802 100644
>> --- a/emacs/notmuch-show.el
>> +++ b/emacs/notmuch-show.el
>> @@ -1835,10 +1835,9 @@ search results instead."
>>    "Archive each message in thread.
>>  
>>  Archive each message currently shown by applying the tag changes
>> -in `notmuch-archive-tags' to each (remove the \"inbox\" tag by
>> -default). If a prefix argument is given, the messages will be
>> -\"unarchived\", i.e. the tag changes in `notmuch-archive-tags'
>> -will be reversed.
>> +in `notmuch-archive-tags' to each. If a prefix argument is given,
>> +the messages will be \"unarchived\", i.e. the tag changes in
>> +`notmuch-archive-tags' will be reversed.
>>  
>>  Note: This command is safe from any race condition of new messages
>>  being delivered to the same thread. It does not archive the
>> @@ -1865,10 +1864,9 @@ buffer."
>>    "Archive the current message.
>>  
>>  Archive the current message by applying the tag changes in
>> -`notmuch-archive-tags' to it (remove the \"inbox\" tag by
>> -default). If a prefix argument is given, the message will be
>> -\"unarchived\", i.e. the tag changes in `notmuch-archive-tags'
>> -will be reversed."
>> +`notmuch-archive-tags' to it. If a prefix argument is given, the
>> +message will be \"unarchived\", i.e. the tag changes in
>> +`notmuch-archive-tags' will be reversed."
>>    (interactive "P")
>>    (when notmuch-archive-tags
>>      (apply 'notmuch-show-tag-message
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH] emacs: remove hardcoded defaults values from docstrings

[David Bremner]

Tomi Ollila <tomi.ollila@iki.fi> writes:

> On Tue, Jun 04 2013, Austin Clements <amdragon@MIT.EDU> wrote:
>
>> I realize this already got pushed, but to me this seems like a
>> reversion for the 95% of users who haven't customized
>> notmuch-archive-tags and just want to know that archiving means
>> removing the inbox tag.  Why is this less confusing?  Users who have
>> customized notmuch-archive-tags will probably know that the "default"
>> doesn't apply to them.

As a side note, this kind of thing is why I started a texinfo manual; I
don't really think reading docstrings is the best intro for a new user.

> Can we have both -- i.e. something like (default: "-inbox") in the 
> docstring. When entering c-h f notmuch-show-archive-thread
> The `notmuch-archive-tags' in text is a hyperlink to the documentation
> of notmuch-archive-tags (which shows its current value)...

I don't really mind Tomi's suggestion, although in practice I think
following the hyperlink is also not so burdensome. I mainly didn't like
the places it mentioned "inbox" without mentioning the variable that
really controls it.

d

Re: [PATCH] emacs: remove hardcoded defaults values from docstrings

[Austin Clements]

Quoth David Bremner on Jun 04 at  8:37 am:
> Tomi Ollila <tomi.ollila@iki.fi> writes:
> 
> > On Tue, Jun 04 2013, Austin Clements <amdragon@MIT.EDU> wrote:
> >
> >> I realize this already got pushed, but to me this seems like a
> >> reversion for the 95% of users who haven't customized
> >> notmuch-archive-tags and just want to know that archiving means
> >> removing the inbox tag.  Why is this less confusing?  Users who have
> >> customized notmuch-archive-tags will probably know that the "default"
> >> doesn't apply to them.
> 
> As a side note, this kind of thing is why I started a texinfo manual; I
> don't really think reading docstrings is the best intro for a new user.
> 
> > Can we have both -- i.e. something like (default: "-inbox") in the 
> > docstring. When entering c-h f notmuch-show-archive-thread
> > The `notmuch-archive-tags' in text is a hyperlink to the documentation
> > of notmuch-archive-tags (which shows its current value)...
> 
> I don't really mind Tomi's suggestion, although in practice I think
> following the hyperlink is also not so burdensome. I mainly didn't like
> the places it mentioned "inbox" without mentioning the variable that
> really controls it.

Right.  I think we should both reference the variable and say what the
default behavior is (there's no reason not to do both).  But isn't
that what these docstrings used to do?

Re: [PATCH] emacs: remove hardcoded defaults values from docstrings

[David Bremner]

Austin Clements <amdragon@MIT.EDU> writes:
>
> Right.  I think we should both reference the variable and say what the
> default behavior is (there's no reason not to do both).  But isn't
> that what these docstrings used to do?

Looking at the old docstrings in notmuch-show.el, I agree they basically
implement Tomi's suggestion.  While I think copying default values of
variables into docstrings creates some minor maintainability traps
(since we then need to remember to look at all the places a variable is
referenced if we change the default value), I'm willing to revert the
patch if people think the tradeoff of better usability is worth it.

It is unfortunate emacs doesn't provide a way to expand the current
value of a variable in the help string, but there we are. It probably
wouldn't be as easy to understand as hand crafted text in any case.

d

Re: [PATCH] emacs: remove hardcoded defaults values from docstrings

[Tomi Ollila]

On Tue, Jun 04 2013, David Bremner <david@tethera.net> wrote:

> Austin Clements <amdragon@MIT.EDU> writes:
>>
>> Right.  I think we should both reference the variable and say what the
>> default behavior is (there's no reason not to do both).  But isn't
>> that what these docstrings used to do?
>
> Looking at the old docstrings in notmuch-show.el, I agree they basically
> implement Tomi's suggestion.  While I think copying default values of
> variables into docstrings creates some minor maintainability traps
> (since we then need to remember to look at all the places a variable is
> referenced if we change the default value), I'm willing to revert the
> patch if people think the tradeoff of better usability is worth it.

Well, revert would be the worst option -- maintainability traps there
and no reference to the variable used ;/

So either what we currently have in repository (and merge Mark's similar
patches) or have both. In addition to my quick suggestion, what is been
seen in notmuch-show.el docstrings this is what `split-string' has:

...
If SEPARATORS is non-nil, it should be a regular expression matching text
which separates, but is not part of, the substrings.  If nil it defaults to
`split-string-default-separators', normally "[ \f\t\n\r\v]+", and
OMIT-NULLS is forced to t.
...

By looking the code, doc hardcoded, defconst split-string-default-separators
-- from maintainability point of view those are close to each other...

IMO what we currently have is OK, unless SomeOne(tm) provides a neat patch
and agrees that maintainability is not really a problem here :D

> It is unfortunate emacs doesn't provide a way to expand the current
> value of a variable in the help string, but there we are. It probably
> wouldn't be as easy to understand as hand crafted text in any case.

We could have placeholders in *.el files and tune byte compiler
to fill in the docstrings >;) Imagine the added bonus we get by
the confusion that causes !


> d

Tomi