`\patchcmd` doesn't allow replacing `##` and gives misleading instructions

[zauguin]

Take the document

\RequirePackage{etoolbox}
\tracingpatches
\newcommand\victim{##}
\patchcmd\victim{##}{}{}{\ERROR}
\stop

This fails with

[debug] tracing \patchcmd on input line 4
[debug] analyzing '\victim'
[debug] ++ control sequence is defined
[debug] ++ control sequence is a macro
[debug] -- nested patching command and parameters in patch
[debug] -> the patching command seems to be nested in the
[debug]    argument to some other command
[debug] -> the patch text seems to contain # characters
[debug] -> either avoid nesting or use # characters with
[debug]    category code 12 in the patch text
[debug] -> simply doubling the # characters will not work
! Undefined control sequence.
<argument> \ERROR 

Here etoolbox seems to mis-analyze the situation:
While the patch text contains a #, it is not nested in any command argument. Also following the advice to give # catcode 12 doesn't work:

\RequirePackage{etoolbox}
\tracingpatches
\newcommand\victim{##}
\begingroup
  \catcode`#=12
  \csname @firstofone\endcsname {%
  \endgroup
  \patchcmd\victim{##}{}{}{\ERROR}
}
\stop

gives the same output.

After weakening the relevant test using

\makeatletter
\protected\long\def\etb@ifhashcheck#1#2#3{#2}
\makeatother

the actual patching seems to work fine though, suggesting that the only issue is the falsely triggered check.

[josephwright]

Wouldn't changing this mean that the case of actually being nested would not be picked up?

[zauguin]

Yes, the \etb@ifhashcheck was not intended as a serious suggestion but more as a demonstration that the error is "only" caused by the checking. I don't see any way to distinguish these inputs right now.

But maybe it would be possible to avoid the requirement to have catcode 12 # in the input, then the test would no longer be necessary and the issue wouldn't occur? You probably already considered such an approach, but maybe the argument could be passed through a macro with 9 arguments to make the #s harmless after scanning? (Along the lines of the PoC in 1dabd29) As this is TeX, there is probably some weird case where this breaks, but currently I can't think of any.

Otherwise it would probably be even a nice improvement if the debugging message would mention that another cause might be a double #. Then the user at least knows why patching fails.