[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#28803: [PATCH] Fixed compiler warnings for advised functions.
From: |
John Williams |
Subject: |
bug#28803: [PATCH] Fixed compiler warnings for advised functions. |
Date: |
Sat, 14 Oct 2017 17:30:46 -0700 |
On Sat, Oct 14, 2017 at 4:47 PM, Noam Postavsky
<npostavs@users.sourceforge.net> wrote:
> John Williams <jrw@pobox.com> writes:
>
>> Oops. Is there anything that can be salvaged from my patch? Aside
>> from fixing the bug, it also adds a unit test and refactors the logic
>> for finding a function's argument list into a separate function
>> that's not part of the help system.
>
> We could add the test, it seems to be passing in emacs-26. Have you
> assigned copyright to Emacs? (It's okay if you haven't, the patch is
> small enough to go in regardless, we would just need to mark it.)
No; how would I go about doing that? The organization of the dev site
is a bit confusing to me.
> I don't think there's much need for moving the function argument
> retrieval out of the help system.
It's not a huge deal to me, but it seems weird that something like
nadvice.el would depend on the help system (which it would in my patch
if I hadn't moved that function--I assume the fix that was already
committed does something similar).
> (By the way, if you've spent some time looking at help-function-arglist,
> perhaps you have some ideas about Bug#26270?
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=26270)
I took a look at your patch, and without having tried running it, it
seems pretty reasonable to me. A unit test of some sort would be nice,
but since there don't seem to be any for help-function-arglist yet,
accepting the patch as-is would leave the unit test situation no worse
than it was before. I'm not a regular contributor, though, so my
opinion is worth what you paid for it.