Subject: Re: bug#24057: 25.1.50; ffap interprets comments beginning with "//" as file path

From: Kaushal Modi <[email-masked]>
Date: Mon, 25 Jul 2016 17:13:51 +0000

On Mon, Jul 25, 2016 at 1:03 PM Eli Zaretskii <[email-masked]> wrote:

> If I understood you correctly, the table you posted didn't cover
> systems which support UNC file names, so you asked me to try on such a
> system.  The above was my response to your request.
>

Thanks. AFAIU, ffap-string-at-point basically deals with just parsing the
string and does not check if a path is valid. So the "////tmp" example in
the table should apply to any use case.

> > The problem with that is .. what is a user has a decorative comment like:
> >
> > ///I would like to
> > ///share foo
> >
> > It is not possible to know if the user liked to use "//" or "///" or
> "////" or .. as the comment prefix. Also it is not possible to know if the
> user meant "/share" or "//share".
>
> IMO, it isn't Emacs's place to second-guess the user in a way that
> prohibits valid use cases.
>

OK.

> > So the best way to make the user's intent clear is by preceding the path
> with a space.
>
> When there's whitespace between the comment leader and the rest, the
> problem I'm talking about doesn't exist.
>

Thanks for confirming that.

> Also thinking that the user meant "//share" in "////share" does not make
> much sense. It's very confusing to count the number of slashes in there.
> What is the user has "/////share" (5 slashes)? Where would we want the
> ffap-string-at-point to guess the comment-starter<>comment boundary then?
>
> If you want to code a backward-compatible solution, then skipping the
> comment-start regexp should do, I think.  Where it stops, ffap should
> start looking for valid file names.
>

I thought about the comment-start variable, but for c-mode

C-h v comment-start gives "/* " and
C-h v comment-start-skip gives "\\(//+\\|/\\*+\\)\\s *"

So it will take "/" followed by 1 or more "/" as comment start.

> > With the current state of ffap-string-at-point, it creates erroneous
> behavior for many people including me. But with the patch, if a user needs
> to put a path like "//share" in a comment for a major mode using "//" as
> comment prefix, all they need to do is use a space to separate the two.
> Also I think that it is very unlikely that someone would have a confusing
> comment like "////share" where the user meant "//share" (or "/share").
>
> I see your point.

Thanks.

> My point is that when we introduce
> backward-incompatible behavior, which makes previously valid use cases
> invalid, such incompatible behavior should initially be opt-in, even
> if it looks to you that the previous behavior made no sense.  I have
> enough gray hair to testify how such decisions in the past turned out
> to be annoyances for some users.  Later, usually years later, we could
> collect user experience and decide to make this the default behavior.
>

If the new behavior is not made the default then we will never know if it
causes user annoyance.

So if you disagree with my suggestion to skip comment-start instead of
> using comment-search-forward, the behavior you propose should IMO be
> off by default.
>

If that's the only way forward, I will add a defcustom in the next patch.
But it doesn't feel right. A defcustom is being created for a very corner
case. Even if I agree with that, it does not feel right to set its default
value so that the buggy behavior is retained by default just to backward
compatibility sake.

How about we add the defcustom and set the default value so that this bug
is fixed. And we wait for people using the master branch to report any
issues caused by this. Also we let emacs-devel know that this defcustom is
going in.
-- 

Kaushal Modi