bug-gnu-emacs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Kaushal Modi
Subject: bug#24057: 25.1.50; ffap interprets comments beginning with "//" as file path
Date: Mon, 25 Jul 2016 17:13:51 +0000

On Mon, Jul 25, 2016 at 1:03 PM Eli Zaretskii <eliz@gnu.org> 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


reply via email to

[Prev in Thread] Current Thread [Next in Thread]