On Fri, Jan 01, 2016 at 05:27:19AM -0600, David Champion wrote:
> Proposed changes to more fully support RFC 2369, "The Use of URLs as
> Meta-Syntax for Core Mail List Commands and their Transport through
> Message Header Fields" -- i.e. the List-*: headers.
> 
> No documentation update is included - will add when there's consensus.

Hi David,

Overall I like the idea.  I have a few comments that I'll just randomly
list here.  If more context is needed, please let me know.

Patch 1 comments:

1. diff -r 9480a363a68a -r cd7be2bfff1c parse.c
  +    if (url_check_scheme (beg) == U_MAILTO)
  +    {
  +      FREE (dst);

You'll probably need to add a /* __FREE_CHECKED__ */ to this line to
make check_sec.sh happy.

2. Is the List-Reply a common header too?  I couldn't find it in the
RFC 2369 document, so I'm just wondering why it's being added and
preferred over List-Post?


Patch 2 comments:

1. It looks like you added the new Esc-L keybinding to main, pager, and
the attach menu.  I only saw code handing it in curs_main.c.  I see the
pager menu will bounce back out to the index menu, but I think it may be
worth adding code inside pager_menu() too, so they are still inside the
message if it was invoked from there.  I don't know if it really makes
sense to have the action in the attach menu though (and it doesn't seem
to be working there in any case).

2. The ListHelp could use an entry for "Archives"

3. In make_entry(), I think you need to add a _() translation marker
around ListActions[num].name so it's translated. (I noticed you flagged
the values with N_ above).

4. The use of the offsetof() in the ListActions.value, while
interesting, makes me a little nervous.  I see how it makes some things
more compact and allows you to combine a select and keyword model
easily.  But it also feels a bit fragile, and different from all the
other menus in mutt.  I'm not vehemently opposed, but was just curious
why you chose that model here.

5. Feel free to add a copyright line in listmenu.c (and also put
yourself in the COPYRIGHT file!)

-- 
Kevin J. McCarthy
GPG Fingerprint: 8975 A9B3 3AA3 7910 385C  5308 ADEF 7684 8031 6BDA
http://www.8t8.us/configs/gpg-key-transition-statement.txt

Attachment: signature.asc
Description: PGP signature

Reply via email to