[PATCH 1/3] lisp/progmodes/etags.el clean up code by removing a temporary

[PATCH 1/3] lisp/progmodes/etags.el clean up code by removing a temporary

[Konstantin Kharlamov]

---
 lisp/progmodes/etags.el | 24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/lisp/progmodes/etags.el b/lisp/progmodes/etags.el
index c2715be5370..05dc7aa1baf 100644
--- a/lisp/progmodes/etags.el
+++ b/lisp/progmodes/etags.el
@@ -1351,7 +1351,7 @@ etags-goto-tag-location
 hits the start of file."
   (let ((startpos (cdr (cdr tag-info)))
 	(line (car (cdr tag-info)))
-	offset found pat)
+	offset pat)
     (if (eq (car tag-info) t)
 	;; Direct file tag.
 	(cond (line (progn (goto-char (point-min))
@@ -1363,7 +1363,6 @@ etags-goto-tag-location
       ;; since just going around the loop once probably
       ;; costs about as much as searching 2000 chars.
       (setq offset 1000
-	    found nil
 	    pat (concat (if (eq selective-display t)
 			    "\\(^\\|\^m\\)" "^")
 			(regexp-quote (car tag-info))))
@@ -1385,19 +1384,16 @@ etags-goto-tag-location
 				    (point)))))
       (or startpos
 	  (setq startpos (point-min)))
-      ;; First see if the tag is right at the specified location.
+
       (goto-char startpos)
-      (setq found (looking-at pat))
-      (while (and (not found)
-		  (progn
-		    (goto-char (- startpos offset))
-		    (not (bobp))))
-	(setq found
-	      (re-search-forward pat (+ startpos offset) t)
-	      offset (* 3 offset)))	; expand search window
-      (or found
-	  (re-search-forward pat nil t)
-	  (user-error "Rerun etags: `%s' not found in %s"
+      (or (looking-at pat) ; Is tag at the specified location?
+          (while (progn
+                   (goto-char (- startpos offset))
+                   (and (not (bobp))
+                        (not (re-search-forward pat (+ startpos offset) t))))
+            (setq offset (* 3 offset))) ; expand search window
+          (re-search-forward pat nil t)
+          (user-error "Rerun etags: `%s' not found in %s"
                       pat buffer-file-name)))
     ;; Position point at the right place
     ;; if the search string matched an extra Ctrl-m at the beginning.
-- 
2.21.0

[PATCH 2/3] lisp/progmodes/etags.el don't (forward-char) as it's overriden next line

[Konstantin Kharlamov]

---
 lisp/progmodes/etags.el | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/lisp/progmodes/etags.el b/lisp/progmodes/etags.el
index 05dc7aa1baf..62637414ef8 100644
--- a/lisp/progmodes/etags.el
+++ b/lisp/progmodes/etags.el
@@ -1395,11 +1395,6 @@ etags-goto-tag-location
           (re-search-forward pat nil t)
           (user-error "Rerun etags: `%s' not found in %s"
                       pat buffer-file-name)))
-    ;; Position point at the right place
-    ;; if the search string matched an extra Ctrl-m at the beginning.
-    (and (eq selective-display t)
-	 (looking-at "\^m")
-	 (forward-char 1))
     (beginning-of-line)))
 
 (defun etags-list-tags (file) ; Doc string?
-- 
2.21.0

[PATCH 3/3] lisp/progmodes/etags.el improve match by string trimming

[Konstantin Kharlamov]

Not only this improves general usability, but also works around existing
bug in anjuta-tags¹

1: https://gitlab.gnome.org/GNOME/anjuta/issues/8
---
 lisp/progmodes/etags.el | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/lisp/progmodes/etags.el b/lisp/progmodes/etags.el
index 62637414ef8..e8b67cf10f4 100644
--- a/lisp/progmodes/etags.el
+++ b/lisp/progmodes/etags.el
@@ -1363,9 +1363,10 @@ etags-goto-tag-location
       ;; since just going around the loop once probably
       ;; costs about as much as searching 2000 chars.
       (setq offset 1000
-	    pat (concat (if (eq selective-display t)
-			    "\\(^\\|\^m\\)" "^")
-			(regexp-quote (car tag-info))))
+            ;; Improve the match by trimming the pattern. It's
+            ;; impossible anyway that 2 tags would only differ by
+            ;; trailing whitespace.
+	    pat (string-trim (regexp-quote (car tag-info))))
       ;; The character position in the tags table is 0-origin and counts CRs.
       ;; Convert it to a 1-origin Emacs character position.
       (when startpos
@@ -1386,8 +1387,7 @@ etags-goto-tag-location
 	  (setq startpos (point-min)))
 
       (goto-char startpos)
-      (or (looking-at pat) ; Is tag at the specified location?
-          (while (progn
+      (or (while (progn
                    (goto-char (- startpos offset))
                    (and (not (bobp))
                         (not (re-search-forward pat (+ startpos offset) t))))
-- 
2.21.0

[PATCH v2] lisp/progmodes/etags.el improve match by string trimming

[Konstantin Kharlamov]

Not only this improves general usability, but also works around existing
bug in anjuta-tags¹

1: https://gitlab.gnome.org/GNOME/anjuta/issues/8
---

Sorry, I didn't test the last change on (looking-at pat) removal, it
still does matching. Fixed here.

 lisp/progmodes/etags.el | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/lisp/progmodes/etags.el b/lisp/progmodes/etags.el
index 62637414ef8..0e0e649a5d5 100644
--- a/lisp/progmodes/etags.el
+++ b/lisp/progmodes/etags.el
@@ -1363,9 +1363,10 @@ etags-goto-tag-location
       ;; since just going around the loop once probably
       ;; costs about as much as searching 2000 chars.
       (setq offset 1000
-	    pat (concat (if (eq selective-display t)
-			    "\\(^\\|\^m\\)" "^")
-			(regexp-quote (car tag-info))))
+            ;; Improve the match by trimming the pattern. It's
+            ;; impossible anyway that 2 tags would only differ by
+            ;; trailing whitespace.
+	    pat (string-trim (regexp-quote (car tag-info))))
       ;; The character position in the tags table is 0-origin and counts CRs.
       ;; Convert it to a 1-origin Emacs character position.
       (when startpos
-- 
2.21.0

Re: [PATCH 2/3] lisp/progmodes/etags.el don't (forward-char) as it's overriden next line

[Eli Zaretskii]

> From: Konstantin Kharlamov <Hi-Angel@yandex.ru>
> Date: Sat, 16 Mar 2019 04:53:13 +0300
> 
> diff --git a/lisp/progmodes/etags.el b/lisp/progmodes/etags.el
> index 05dc7aa1baf..62637414ef8 100644
> --- a/lisp/progmodes/etags.el
> +++ b/lisp/progmodes/etags.el
> @@ -1395,11 +1395,6 @@ etags-goto-tag-location
>            (re-search-forward pat nil t)
>            (user-error "Rerun etags: `%s' not found in %s"
>                        pat buffer-file-name)))
> -    ;; Position point at the right place
> -    ;; if the search string matched an extra Ctrl-m at the beginning.
> -    (and (eq selective-display t)
> -	 (looking-at "\^m")
> -	 (forward-char 1))
>      (beginning-of-line)))

Did you actually try this change with selective-display in effect?
Because I'm not sure it is correct in that case.

Thanks.

Re: [PATCH v2] lisp/progmodes/etags.el improve match by string trimming

[Eli Zaretskii]

> From: Konstantin Kharlamov <Hi-Angel@yandex.ru>
> Date: Sat, 16 Mar 2019 05:13:33 +0300
> 
> Not only this improves general usability, but also works around existing
> bug in anjuta-tags¹
> 
> 1: https://gitlab.gnome.org/GNOME/anjuta/issues/8
> ---
> 
> Sorry, I didn't test the last change on (looking-at pat) removal, it
> still does matching. Fixed here.

Please explain more about the original problem.  The proposed change
looks like it removes the support for selective-display, or am I
missing something?

IOW, why is the regexp including the CR character a problem?

Thanks.

Re: [PATCH v2] lisp/progmodes/etags.el improve match by string trimming

[Konstantin Kharlamov]

В Сб, мар 16, 2019 at 3:46 ПП (PM), Eli Zaretskii 
<eliz@gnu.org> написал:
>>  From: Konstantin Kharlamov <Hi-Angel@yandex.ru>
>>  Date: Sat, 16 Mar 2019 05:13:33 +0300
>> 
>>  Not only this improves general usability, but also works around 
>> existing
>>  bug in anjuta-tags¹
>> 
>>  1: https://gitlab.gnome.org/GNOME/anjuta/issues/8
>>  ---
>> 
>>  Sorry, I didn't test the last change on (looking-at pat) removal, it
>>  still does matching. Fixed here.
> 
> Please explain more about the original problem.

The pattern that this functions searches for determines the tag 
uniquely. But here's a catch: no programming language creates distinc 
entities (ones that end up in the tag), based only on trailing space. 
I.e. "foo()" and "    foo()   " always refer to the same thing.

So this change gives 2 improvements:
1. Emacs gonna stop failing finding a tag when someone simply 
reindented the sources
2. As a side effect, this works around a problem, when a 
tags-generating program trimmed the whitespace (the commit has a link 
to anjuta-tags bugreport about that).

 > The proposed change
> looks like it removes the support for selective-display, or am I
> missing something?
> 
> IOW, why is the regexp including the CR character a problem?
> 
> Thanks.

"CR" is part of the whitespace, so per description above comparison to 
it is simply unnecessary.

Re: [PATCH 2/3] lisp/progmodes/etags.el don't (forward-char) as it's overriden next line

[Konstantin Kharlamov]

В Сб, мар 16, 2019 at 3:43 ПП (PM), Eli Zaretskii 
<eliz@gnu.org> написал:
>>  From: Konstantin Kharlamov <Hi-Angel@yandex.ru>
>>  Date: Sat, 16 Mar 2019 04:53:13 +0300
>> 
>>  diff --git a/lisp/progmodes/etags.el b/lisp/progmodes/etags.el
>>  index 05dc7aa1baf..62637414ef8 100644
>>  --- a/lisp/progmodes/etags.el
>>  +++ b/lisp/progmodes/etags.el
>>  @@ -1395,11 +1395,6 @@ etags-goto-tag-location
>>             (re-search-forward pat nil t)
>>             (user-error "Rerun etags: `%s' not found in %s"
>>                         pat buffer-file-name)))
>>  -    ;; Position point at the right place
>>  -    ;; if the search string matched an extra Ctrl-m at the 
>> beginning.
>>  -    (and (eq selective-display t)
>>  -	 (looking-at "\^m")
>>  -	 (forward-char 1))
>>       (beginning-of-line)))
> 
> Did you actually try this change with selective-display in effect?
> Because I'm not sure it is correct in that case.
> 
> Thanks.

Hmm… I will try a bit later, but the code seems really 
straightforward:
	1. if current character is ^M, then step forward
	2. step to the beginnig of the line

The 2 undoes 1.

Re: [PATCH 2/3] lisp/progmodes/etags.el don't (forward-char) as it's overriden next line

[Stefan Monnier]

> Hmm… I will try a bit later, but the code seems really straightforward:
> 	1. if current character is ^M, then step forward
> 	2. step to the beginnig of the line
>
> The 2 undoes 1.

When selective-display is t I think 2 does not undo 1.


        Stefan "who'd be happy to see the t value of selective-display
                officially deprecated and obsoleted"

Re: [PATCH 2/3] lisp/progmodes/etags.el don't (forward-char) as it's overriden next line

[Eli Zaretskii]

> Date: Sat, 16 Mar 2019 18:42:44 +0300
> From: Konstantin Kharlamov <hi-angel@yandex.ru>
> Cc: emacs-devel@gnu.org
> 
> >>  -    (and (eq selective-display t)
> >>  -	 (looking-at "\^m")
> >>  -	 (forward-char 1))
> >>       (beginning-of-line)))
> > 
> > Did you actually try this change with selective-display in effect?
> > Because I'm not sure it is correct in that case.
> > 
> > Thanks.
> 
> Hmm… I will try a bit later, but the code seems really 
> straightforward:
> 	1. if current character is ^M, then step forward
> 	2. step to the beginnig of the line
> 
> The 2 undoes 1.

^M has special meaning in selective-display mode.

Re: [PATCH v2] lisp/progmodes/etags.el improve match by string trimming

[Eli Zaretskii]

> Date: Sat, 16 Mar 2019 18:38:32 +0300
> From: Konstantin Kharlamov <hi-angel@yandex.ru>
> Cc: emacs-devel@gnu.org
> 
> "CR" is part of the whitespace, so per description above comparison to 
> it is simply unnecessary.

CR is treated specially when selective-display is in effect, it's not
just any whitespace.

Re: [PATCH 2/3] lisp/progmodes/etags.el don't (forward-char) as it's overriden next line

[Konstantin Kharlamov]

В Сб, мар 16, 2019 at 7:26 ПП (PM), Eli Zaretskii 
<eliz@gnu.org> написал:
>>  Date: Sat, 16 Mar 2019 18:42:44 +0300
>>  From: Konstantin Kharlamov <hi-angel@yandex.ru>
>>  Cc: emacs-devel@gnu.org
>> 
>>  >>  -    (and (eq selective-display t)
>>  >>  -	 (looking-at "\^m")
>>  >>  -	 (forward-char 1))
>>  >>       (beginning-of-line)))
>>  >
>>  > Did you actually try this change with selective-display in effect?
>>  > Because I'm not sure it is correct in that case.
>>  >
>>  > Thanks.
>> 
>>  Hmm… I will try a bit later, but the code seems really
>>  straightforward:
>>  	1. if current character is ^M, then step forward
>>  	2. step to the beginnig of the line
>> 
>>  The 2 undoes 1.
> 
> ^M has special meaning in selective-display mode.

Thanks, so, I just tried playing with the mode, and I haven't even 
found ^M anywhere. I wonder if the ^M being added by the mode is in the 
past, that just no longer happening.

Specifically, I did:

1. wrote in scratch buffer
test
 test
  test
2. evaluated (set-selective-display 1) which transformed the look of 
the text to
test...
3.α Tried searching for ^M by pressing C-s C-q C-m. It's failing to 
find anything.
3.β Put the caret before the triple dot, as in "test|...", and 
evaluated (char-after (point)) and (char-after (+ 1 (point))). The 
first returns "C-j" character, i.e. a newline; the second returns 
"space character". They're both the exact characters that actually are 
in text, as if I didn't enable selective-display.

So, even disregarding my patches (because I haven't managed to test 
this possible corner-case), the need for ^M workarounds is just no 
longer needed.

Re: [PATCH 2/3] lisp/progmodes/etags.el don't (forward-char) as it's overriden next line

[Konstantin Kharlamov]

В Вс, мар 17, 2019 at 12:12 ДП (AM), Konstantin Kharlamov 
<hi-angel@yandex.ru> написал:
> 
> 
> В Сб, мар 16, 2019 at 7:26 ПП (PM), Eli Zaretskii 
> <eliz@gnu.org> написал:
>>>  Date: Sat, 16 Mar 2019 18:42:44 +0300
>>>  From: Konstantin Kharlamov <hi-angel@yandex.ru>
>>>  Cc: emacs-devel@gnu.org
>>> 
>>>  >>  -    (and (eq selective-display t)
>>>  >>  -	 (looking-at "\^m")
>>>  >>  -	 (forward-char 1))
>>>  >>       (beginning-of-line)))
>>>  >
>>>  > Did you actually try this change with selective-display in 
>>> effect?
>>>  > Because I'm not sure it is correct in that case.
>>>  >
>>>  > Thanks.
>>> 
>>>  Hmm… I will try a bit later, but the code seems really
>>>  straightforward:
>>>  	1. if current character is ^M, then step forward
>>>  	2. step to the beginnig of the line
>>> 
>>>  The 2 undoes 1.
>> 
>> ^M has special meaning in selective-display mode.
> 
> Thanks, so, I just tried playing with the mode, and I haven't even 
> found ^M anywhere. I wonder if the ^M being added by the mode is in 
> the past, that just no longer happening.
> 
> Specifically, I did:
> 
> 1. wrote in scratch buffer
> test
> test
>  test
> 2. evaluated (set-selective-display 1) which transformed the look of 
> the text to
> test...
> 3.α Tried searching for ^M by pressing C-s C-q C-m. It's failing to 
> find anything.
> 3.β Put the caret before the triple dot, as in "test|...", and 
> evaluated (char-after (point)) and (char-after (+ 1 (point))). The 
> first returns "C-j" character, i.e. a newline; the second returns 
> "space character". They're both the exact characters that actually 
> are in text, as if I didn't enable selective-display.
> 
> So, even disregarding my patches (because I haven't managed to test 
> this possible corner-case), the need for ^M workarounds is just no 
> longer needed.

That said, would be nice if somebody could confirm my findings, because 
I can not find whether my current Emacs built from vanilla branch or 
the harf-buzz one; and I'm a bit worrying that this could've been 
change specific to harf-buzz branch.

[PATCH v3] lisp/progmodes/etags.el improve match by string trimming

[Konstantin Kharlamov]

Not only this improves general usability, but also works around existing
bug in anjuta-tags¹

1: https://gitlab.gnome.org/GNOME/anjuta/issues/8
---

v3: swap regexp-quote and string-trim, that works faster

 lisp/progmodes/etags.el | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/lisp/progmodes/etags.el b/lisp/progmodes/etags.el
index 62637414ef8..d5162f01e47 100644
--- a/lisp/progmodes/etags.el
+++ b/lisp/progmodes/etags.el
@@ -1363,9 +1363,10 @@ etags-goto-tag-location
       ;; since just going around the loop once probably
       ;; costs about as much as searching 2000 chars.
       (setq offset 1000
-	    pat (concat (if (eq selective-display t)
-			    "\\(^\\|\^m\\)" "^")
-			(regexp-quote (car tag-info))))
+            ;; Improve the match by trimming the pattern. It's
+            ;; impossible anyway that 2 tags would only differ by
+            ;; trailing whitespace.
+	    pat (regexp-quote (string-trim (car tag-info))))
       ;; The character position in the tags table is 0-origin and counts CRs.
       ;; Convert it to a 1-origin Emacs character position.
       (when startpos
-- 
2.21.0

Re: [PATCH 2/3] lisp/progmodes/etags.el don't (forward-char) as it's overriden next line

[Eli Zaretskii]

> Date: Sun, 17 Mar 2019 00:12:36 +0300
> From: Konstantin Kharlamov <hi-angel@yandex.ru>
> Cc: emacs-devel@gnu.org
> 
> > ^M has special meaning in selective-display mode.
> 
> Thanks, so, I just tried playing with the mode, and I haven't even 
> found ^M anywhere. I wonder if the ^M being added by the mode is in the 
> past, that just no longer happening.
> 
> Specifically, I did:
> 
> 1. wrote in scratch buffer
> test
>  test
>   test
> 2. evaluated (set-selective-display 1) which transformed the look of 
> the text to
> test...
> 3.α Tried searching for ^M by pressing C-s C-q C-m. It's failing to 
> find anything.

The ^M character is what causes that "..." ellipsis.  You cannot
search for it interactively because Emacs prevents that, but the
character is there in the buffer text.

Re: [PATCH 2/3] lisp/progmodes/etags.el don't (forward-char) as it's overriden next line

[Konstantin Kharlamov]

В Вс, мар 17, 2019 at 6:36 ДП (AM), Eli Zaretskii 
<eliz@gnu.org> написал:
>>  Date: Sun, 17 Mar 2019 00:12:36 +0300
>>  From: Konstantin Kharlamov <hi-angel@yandex.ru>
>>  Cc: emacs-devel@gnu.org
>> 
>>  > ^M has special meaning in selective-display mode.
>> 
>>  Thanks, so, I just tried playing with the mode, and I haven't even
>>  found ^M anywhere. I wonder if the ^M being added by the mode is in 
>> the
>>  past, that just no longer happening.
>> 
>>  Specifically, I did:
>> 
>>  1. wrote in scratch buffer
>>  test
>>   test
>>    test
>>  2. evaluated (set-selective-display 1) which transformed the look of
>>  the text to
>>  test...
>>  3.α Tried searching for ^M by pressing C-s C-q C-m. It's failing to
>>  find anything.
> 
> The ^M character is what causes that "..." ellipsis.  You cannot
> search for it interactively because Emacs prevents that, but the
> character is there in the buffer text.

Okay, well, I couldn't find it by evaluating (re-search-forward "^M") 
either.

So, given the character can't be inspected, the old code couldn't even 
do anything :)

Re: [PATCH 2/3] lisp/progmodes/etags.el don't (forward-char) as it's overriden next line

[Eli Zaretskii]

> Date: Sun, 17 Mar 2019 06:41:41 +0300
> From: Konstantin Kharlamov <hi-angel@yandex.ru>
> Cc: emacs-devel@gnu.org
> 
> > The ^M character is what causes that "..." ellipsis.  You cannot
> > search for it interactively because Emacs prevents that, but the
> > character is there in the buffer text.
> 
> Okay, well, I couldn't find it by evaluating (re-search-forward "^M") 
> either.
> 
> So, given the character can't be inspected, the old code couldn't even 
> do anything :)

I suggest to read the node "Selective Display" in the ELisp manual
about this feature, and how the ^M characters come into play.

Re: [PATCH 2/3] lisp/progmodes/etags.el don't (forward-char) as it's overriden next line

[Stefan Monnier]

> I suggest to read the node "Selective Display" in the ELisp manual
> about this feature, and how the ^M characters come into play.

Could we declare the t value of selective-display deprecated
(recommending the use of the `invisible` property instead)?


        Stefan

Re: [PATCH 2/3] lisp/progmodes/etags.el don't (forward-char) as it's overriden next line

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Sun, 17 Mar 2019 11:52:53 -0400
> 
> Could we declare the t value of selective-display deprecated
> (recommending the use of the `invisible` property instead)?

We already do, see the node "Selective Display".  And you already
asked this several years ago ;-)

  http://lists.gnu.org/archive/html/emacs-devel/2016-01/msg00371.html

Re: [PATCH 2/3] lisp/progmodes/etags.el don't (forward-char) as it's overriden next line

[Stefan Monnier]

> We already do, see the node "Selective Display".  And you already
> asked this several years ago ;-)
>   http://lists.gnu.org/archive/html/emacs-devel/2016-01/msg00371.html

Great!
So now let's get rid of all uses of it.

AFAICT within Emacs's `master` it was only used in Dired and
Ebrowse, so I just pushed a change to get rid of it in Dired.


        Stefan

Re: [PATCH 2/3] lisp/progmodes/etags.el don't (forward-char) as it's overriden next line

[Konstantin Kharlamov]

В Вс, мар 17, 2019 at 6:17 ПП (PM), Eli Zaretskii 
<eliz@gnu.org> написал:
>>  Date: Sun, 17 Mar 2019 06:41:41 +0300
>>  From: Konstantin Kharlamov <hi-angel@yandex.ru>
>>  Cc: emacs-devel@gnu.org
>> 
>>  > The ^M character is what causes that "..." ellipsis.  You cannot
>>  > search for it interactively because Emacs prevents that, but the
>>  > character is there in the buffer text.
>> 
>>  Okay, well, I couldn't find it by evaluating (re-search-forward 
>> "^M")
>>  either.
>> 
>>  So, given the character can't be inspected, the old code couldn't 
>> even
>>  do anything :)
> 
> I suggest to read the node "Selective Display" in the ELisp manual
> about this feature, and how the ^M characters come into play.

Ok, the docs say that every newline in "invisible part" is replaced 
with ^M.

But this doesn't change the fact this ^M can't be inspected, in 
particular by (looking-at "\^m") that the 2-nd patch removes. I just 
tested that.

Oh well, whatever, I'm not really motivated to discuss that, let's just 
drop 2-nd patch. Are patches 1 and 3 okay?

Re: [PATCH 2/3] lisp/progmodes/etags.el don't (forward-char) as it's overriden next line

[Eli Zaretskii]

> Date: Sun, 17 Mar 2019 22:06:26 +0300
> From: Konstantin Kharlamov <hi-angel@yandex.ru>
> Cc: emacs-devel@gnu.org
> 
> Are patches 1 and 3 okay?

Part 3 has the same problem as part 2, and I think I already responded
to it.

Part 1 I need to look closer, will do soon.

Thanks.

Re: [PATCH 2/3] lisp/progmodes/etags.el don't (forward-char) as it's overriden next line

[Konstantin Kharlamov]

В Вс, мар 17, 2019 at 10:22 ПП (PM), Eli Zaretskii 
<eliz@gnu.org> написал:
>>  Date: Sun, 17 Mar 2019 22:06:26 +0300
>>  From: Konstantin Kharlamov <hi-angel@yandex.ru>
>>  Cc: emacs-devel@gnu.org
>> 
>>  Are patches 1 and 3 okay?
> 
> Part 3 has the same problem as part 2, and I think I already responded
> to it.

Ah, sorry, I thought that was a general comment, i.e. a reply to my 
mail but not specific to the patch.

Okay, so: ^M appears in selective-display-mode only in the end of a 
line. So, matching the ^M have been relevant before the patch: we 
wanted to match beginning of the line, which is defined by ^M character.

But after my patch applied we do not match beginning of the line 
anymore, at all, not in any mode. We only match the exact tag, with 
trimmed space. I.e. matching for ^ in usual mode and for ^M in 
selective-display-mode is no longer needed.

Re: [PATCH 2/3] lisp/progmodes/etags.el don't (forward-char) as it's overriden next line

[Eli Zaretskii]

> Date: Sun, 17 Mar 2019 22:29:55 +0300
> From: Konstantin Kharlamov <hi-angel@yandex.ru>
> Cc: emacs-devel@gnu.org
> 
> Okay, so: ^M appears in selective-display-mode only in the end of a 
> line.

No, ^M _replaces_ newline characters, so what was previously several
lines is now a single long line.

Re: [PATCH 2/3] lisp/progmodes/etags.el don't (forward-char) as it's overriden next line

[Konstantin Kharlamov]

В Вс, мар 17, 2019 at 11:21 ПП (PM), Eli Zaretskii 
<eliz@gnu.org> написал:
>>  Date: Sun, 17 Mar 2019 22:29:55 +0300
>>  From: Konstantin Kharlamov <hi-angel@yandex.ru>
>>  Cc: emacs-devel@gnu.org
>> 
>>  Okay, so: ^M appears in selective-display-mode only in the end of a
>>  line.
> 
> No, ^M _replaces_ newline characters, so what was previously several
> lines is now a single long line.

Sure, but after the patch we don't match the whole line anymore. We 
only match the tag. We don't care if it's at the beginning of the line 
or in the middle of it. We only look for the tag. So we don't care if 
line has a newline character or ^M.

Re: [PATCH 2/3] lisp/progmodes/etags.el don't (forward-char) as it's overriden next line

[Eli Zaretskii]

> Date: Sun, 17 Mar 2019 23:27:30 +0300
> From: Konstantin Kharlamov <hi-angel@yandex.ru>
> Cc: emacs-devel@gnu.org
> 
> >>  Okay, so: ^M appears in selective-display-mode only in the end of a
> >>  line.
> > 
> > No, ^M _replaces_ newline characters, so what was previously several
> > lines is now a single long line.
> 
> Sure, but after the patch we don't match the whole line anymore. We 
> only match the tag. We don't care if it's at the beginning of the line 
> or in the middle of it. We only look for the tag. So we don't care if 
> line has a newline character or ^M.

But that's incorrect: a tag in a tags table always begins at the
beginning of a line, so it must be a search anchored at the beginning
of the line.  And in selective-display mode ^M counts as the beginning
of a line.

Re: [PATCH 2/3] lisp/progmodes/etags.el don't (forward-char) as it's overriden next line

[Konstantin Kharlamov]

В Вс, мар 17, 2019 at 11:40 ПП (PM), Eli Zaretskii 
<eliz@gnu.org> написал:
>>  Date: Sun, 17 Mar 2019 23:27:30 +0300
>>  From: Konstantin Kharlamov <hi-angel@yandex.ru>
>>  Cc: emacs-devel@gnu.org
>> 
>>  >>  Okay, so: ^M appears in selective-display-mode only in the end 
>> of a
>>  >>  line.
>>  >
>>  > No, ^M _replaces_ newline characters, so what was previously 
>> several
>>  > lines is now a single long line.
>> 
>>  Sure, but after the patch we don't match the whole line anymore. We
>>  only match the tag. We don't care if it's at the beginning of the 
>> line
>>  or in the middle of it. We only look for the tag. So we don't care 
>> if
>>  line has a newline character or ^M.
> 
> But that's incorrect: a tag in a tags table always begins at the
> beginning of a line, so it must be a search anchored at the beginning
> of the line.  And in selective-display mode ^M counts as the beginning
> of a line.

Great, now that we established that part, to reply your question why we 
don't care about the beginnging of the line let me quote my other mail:

 > The pattern that this functions searches for determines the tag 
uniquely. But here's a catch: no programming language creates distinc 
entities (ones that end up in the tag), based only on trailing space. 
I.e. "foo()" and " foo() " always refer to the same thing.
 >
 > So this change gives 2 improvements:
 > 1. Emacs gonna stop failing finding a tag when someone simply 
reindented the sources
 > 2. As a side effect, this works around a problem, when a 
tags-generating program trimmed the whitespace (the commit has a link 
to anjuta-tags bugreport about that).

Eradicating selective-display == t (was: [PATCH 2/3] lisp/progmodes/etags.el don't (forward-char) as it's overriden next line)

[Stefan Monnier]

> AFAICT within Emacs's `master` it was only used in Dired and
> Ebrowse, so I just pushed a change to get rid of it in Dired.

I have a patch for Ebrowse as well, but I can't test it, cause I have no
C++ code at hand to play with.

Can someone help me test the patch below?


        Stefan


diff --git a/lisp/progmodes/ebrowse.el b/lisp/progmodes/ebrowse.el
index f501f7353b..5bdc7b2bfb 100644
--- a/lisp/progmodes/ebrowse.el
+++ b/lisp/progmodes/ebrowse.el
@@ -1,4 +1,4 @@
-;;; ebrowse.el --- Emacs C++ class browser & tags facility
+;;; ebrowse.el --- Emacs C++ class browser & tags facility  -*- lexical-binding:t -*-
 
 ;; Copyright (C) 1992-2019 Free Software Foundation, Inc.
 
@@ -233,30 +233,12 @@ ebrowse-position
     found))
 
 
-(defmacro ebrowse-output (&rest body)
-  "Eval BODY with a writable current buffer.
-Preserve buffer's modified state."
-  (declare (indent 0) (debug t))
-  (let ((modified (make-symbol "--ebrowse-output--")))
-    `(let (buffer-read-only (,modified (buffer-modified-p)))
-       (unwind-protect
-	   (progn ,@body)
-	 (set-buffer-modified-p ,modified)))))
-
-
 (defmacro ebrowse-ignoring-completion-case (&rest body)
   "Eval BODY with `completion-ignore-case' bound to t."
   (declare (indent 0) (debug t))
   `(let ((completion-ignore-case t))
      ,@body))
 
-(defmacro ebrowse-save-selective (&rest body)
-  "Eval BODY with `selective-display' restored at the end."
-  (declare (indent 0) (debug t))
-  ;; FIXME: Don't use selective-display.
-  `(let ((selective-display selective-display))
-     ,@body))
-
 (defmacro ebrowse-for-all-trees (spec &rest body)
   "For all trees in SPEC, eval BODY."
   (declare (indent 1) (debug ((sexp form) body)))
@@ -303,7 +285,7 @@ ebrowse-rename-buffer
 (defun ebrowse-trim-string (string)
   "Return a copy of STRING with leading white space removed.
 Replace sequences of newlines with a single space."
-  (when (string-match "^[ \t\n\r]+" string)
+  (when (string-match "^[ \t\n]+" string)
     (setq string (substring string (match-end 0))))
   (cl-loop while (string-match "[\n]+" string)
            finally return string do
@@ -688,7 +670,7 @@ ebrowse-files-list
   "Return a list containing all files mentioned in a tree.
 MARKED-ONLY non-nil means include marked classes only."
   (let (list)
-    (maphash (lambda (file _dummy) (setq list (cons file list)))
+    (maphash (lambda (file _dummy) (push file list))
 	     (ebrowse-files-table marked-only))
     list))
 
@@ -865,7 +847,7 @@ ebrowse-read
     ;; Read Lisp objects.  Temporarily increase `gc-cons-threshold' to
     ;; prevent a GC that would not free any memory.
     (let ((gc-cons-threshold 2000000))
-      (while (not (progn (skip-chars-forward " \t\n\r") (eobp)))
+      (while (not (progn (skip-chars-forward " \t\n") (eobp)))
 	(let* ((root (read (current-buffer)))
 	       (old-root-ptr (ebrowse-class-in-tree root tree)))
 	  (ebrowse-show-progress "Reading data" (null tree))
@@ -996,7 +978,6 @@ ebrowse-insert-supers
                               (ebrowse-qualified-class-name
                                (ebrowse-ts-class (car subclass)))
                               classes)
-                    as next = nil
                     do
                     ;; Replace the subclass tree with the one found in
                     ;; CLASSES if there is already an entry for that class
@@ -1096,8 +1077,7 @@ ebrowse-tree-mode
     (set (make-local-variable 'ebrowse--frozen-flag) nil)
     (setq mode-line-buffer-identification ident)
     (setq buffer-read-only t)
-    (setq selective-display t)
-    (setq selective-display-ellipses t)
+    (add-to-invisibility-spec '(ebrowse . t))
     (set (make-local-variable 'revert-buffer-function)
          #'ebrowse-revert-tree-buffer-from-file)
     (set (make-local-variable 'ebrowse--header) header)
@@ -1107,7 +1087,7 @@ ebrowse-tree-mode
          (and tree (ebrowse-build-tree-obarray tree)))
     (set (make-local-variable 'ebrowse--frozen-flag) nil)
 
-    (add-hook 'write-file-functions 'ebrowse-write-file-hook-fn nil t)
+    (add-hook 'write-file-functions #'ebrowse-write-file-hook-fn nil t)
     (modify-syntax-entry ?_ (char-to-string (char-syntax ?a)))
     (when tree
       (ebrowse-redraw-tree)
@@ -1184,7 +1164,7 @@ ebrowse-toggle-mark-at-point
       ;; by a regexp replace over the whole buffer. The reason for this
       ;; is that classes might have multiple base classes. If this is
       ;; the case, they are displayed more than once in the tree.
-      (ebrowse-output
+      (with-silent-modifications
 	(cl-loop
          for tree in to-change
          as regexp = (concat "^.*\\b"
@@ -1213,7 +1193,7 @@ ebrowse-redraw-marks
   "Display class marker signs in the tree between START and END."
   (interactive)
   (save-excursion
-    (ebrowse-output
+    (with-silent-modifications
       (catch 'end
 	(goto-char (point-min))
 	(dolist (root ebrowse--tree)
@@ -1242,8 +1222,8 @@ ebrowse-show-file-name-at-point
 With PREFIX, insert that many filenames."
   (interactive "p")
   (unless ebrowse--show-file-names-flag
-    (ebrowse-output
-      (dotimes (i prefix)
+    (with-silent-modifications
+      (dotimes (_ prefix)
 	(let ((tree (ebrowse-tree-at-point))
 	      start
 	      file-name-existing)
@@ -1393,6 +1373,18 @@ ebrowse-pop-to-browser-buffer
 
 
 \f
+;;; Functions to hide/unhide text
+
+(defun ebrowse--hidden-p (&optional pos)
+  (eq (get-char-property (or pos (point)) 'invisible) 'ebrowse))
+
+(defun ebrowse--hide (start end)
+  (put-text-property start end 'invisible 'ebrowse))
+
+(defun ebrowse--unhide (start end)
+  ;; FIXME: This also removes other invisible properties!
+  (remove-text-properties start end '(invisible)))
+
 ;;; Misc tree buffer commands
 
 (defun ebrowse-set-tree-indentation ()
@@ -1418,16 +1410,14 @@ ebrowse-read-class-name-and-go
       (setf class
 	    (completing-read "Goto class: "
 			     (ebrowse-tree-obarray-as-alist) nil t)))
-    (ebrowse-save-selective
-      (goto-char (point-min))
-      (widen)
-      (setf selective-display nil)
-      (setq ebrowse--last-regexp (concat "\\b" class "\\b"))
-      (if (re-search-forward ebrowse--last-regexp nil t)
-	  (progn
-	    (goto-char (match-beginning 0))
-	    (ebrowse-unhide-base-classes))
-	(error "Not found")))))
+    (goto-char (point-min))
+    (widen)
+    (setq ebrowse--last-regexp (concat "\\b" class "\\b"))
+    (if (re-search-forward ebrowse--last-regexp nil t)
+	(progn
+	  (goto-char (match-beginning 0))
+	  (ebrowse-unhide-base-classes))
+      (error "Not found"))))
 
 
 \f
@@ -1556,7 +1546,7 @@ ebrowse-view-exit-fn
       (setq original-frame-configuration ebrowse--frame-configuration
 	    exit-action ebrowse--view-exit-action))
     ;; Delete the frame in which we viewed.
-    (mapc 'delete-frame
+    (mapc #'delete-frame
 	  (cl-loop for frame in (frame-list)
                    when (not (assq frame original-frame-configuration))
                    collect frame))
@@ -1610,9 +1600,7 @@ ebrowse-view/find-file-and-search-pattern
   (cond (view
 	 (setf ebrowse-temp-position-to-view struc
 	       ebrowse-temp-info-to-view info)
-	 (unless (boundp 'view-mode-hook)
-	   (setq view-mode-hook nil))
-	 (push 'ebrowse-find-pattern view-mode-hook)
+         (add-hook 'view-mode-hook #'ebrowse-find-pattern)
 	 (pcase where
 	   ('other-window (view-file-other-window file))
 	   ('other-frame  (ebrowse-view-file-other-frame file))
@@ -1676,7 +1664,7 @@ ebrowse-pp-define-regexp
 
 INFO is a list (TREE-HEADER TREE-OR-MEMBER MEMBER-LIST)."
   (unless position
-    (pop view-mode-hook)
+    (remove-hook 'view-mode-hook #'ebrowse-find-pattern)
     (setf viewing t
 	  position ebrowse-temp-position-to-view
 	  info ebrowse-temp-info-to-view))
@@ -1685,7 +1673,7 @@ ebrowse-pp-define-regexp
 	 (start (ebrowse-bs-point position))
 	 (offset 100)
 	 found)
-    (pcase-let ((`(,header ,class-or-member ,member-list) info))
+    (pcase-let ((`(,_header ,class-or-member ,member-list) info))
       ;; If no pattern is specified, construct one from the member name.
       (when (stringp pattern)
 	(setq pattern (concat "^.*" (regexp-quote pattern))))
@@ -1749,7 +1737,7 @@ ebrowse-redraw-tree
   (interactive)
   (or quietly (message "Displaying..."))
   (save-excursion
-    (ebrowse-output
+    (with-silent-modifications
       (erase-buffer)
       (ebrowse-draw-tree-fn)))
   (ebrowse-update-tree-buffer-mode-line)
@@ -1816,7 +1804,8 @@ ebrowse-set-mark-props
                    (nconc (copy-sequence (ebrowse-ts-subclasses tree)) stack2)
                    stack1
                    (nconc (make-list (length (ebrowse-ts-subclasses tree))
-                                     (1+ level)) stack1)))))
+                                     (1+ level))
+                          stack1)))))
 
 
 \f
@@ -1844,69 +1833,60 @@ ebrowse-expand-all
   "Expand or fold all trees in the buffer.
 COLLAPSE non-nil means fold them."
   (interactive "P")
-  (let ((line-end  (if collapse "^\n" "^\r"))
-	(insertion (if collapse "\r"  "\n")))
-    (ebrowse-output
+  (with-silent-modifications
+    (if (not collapse)
+        (ebrowse--unhide (point-min) (point-max))
       (save-excursion
 	(goto-char (point-min))
-	(while (not (progn (skip-chars-forward line-end) (eobp)))
-	  (when (or (not collapse)
-		    (looking-at "\n "))
-	    (delete-char 1)
-	    (insert insertion))
-	  (when collapse
-	    (skip-chars-forward "\n ")))))))
+	(while (progn (end-of-line) (not (eobp)))
+	  (when (looking-at "\n ")
+            (ebrowse--hide (point) (line-end-position 2)))
+	  (skip-chars-forward "\n "))))))
 
 
 (defun ebrowse-unhide-base-classes ()
   "Unhide the line the cursor is on and all base classes."
-  (ebrowse-output
+  (with-silent-modifications
     (save-excursion
       (let (indent last-indent)
-	(skip-chars-backward "^\r\n")
-	(when (not (looking-at "[\r\n][^ \t]"))
-	  (skip-chars-forward "\r\n \t")
+	(forward-line 0)
+	(when (not (looking-at "\n[^ \t]"))
+	  (skip-chars-forward "\n \t")
 	  (while (and (or (null last-indent) ;first time
 			  (> indent 1))	;not root class
-		      (re-search-backward "[\r\n][ \t]*" nil t))
+		      (re-search-backward "\n[ \t]*" nil t))
 	    (setf indent (- (match-end 0)
 			    (match-beginning 0)))
 	    (when (or (null last-indent)
 		      (< indent last-indent))
 	      (setf last-indent indent)
-	      (when (looking-at "\r")
-		(delete-char 1)
-		(insert 10)))
-	    (backward-char 1)))))))
+	      (when (ebrowse--hidden-p)
+                (ebrowse--unhide (point) (line-end-position 2))))))))))
 
 
 (defun ebrowse-hide-line (collapse)
   "Hide/show a single line in the tree.
 COLLAPSE non-nil means hide."
-  (save-excursion
-    (ebrowse-output
-      (skip-chars-forward "^\r\n")
-      (delete-char 1)
-      (insert (if collapse 13 10)))))
+  (with-silent-modifications
+    (funcall (if collapse #'ebrowse--hide #'ebrowse--unhide)
+             (line-end-position) (line-end-position 2))))
 
 
 (defun ebrowse-collapse-fn (collapse)
   "Collapse or expand a branch of the tree.
 COLLAPSE non-nil means collapse the branch."
-  (ebrowse-output
+  (with-silent-modifications
     (save-excursion
       (beginning-of-line)
       (skip-chars-forward "> \t")
       (let ((indentation (current-column)))
 	(while (and (not (eobp))
 		    (save-excursion
-		      (skip-chars-forward "^\r\n")
-		      (goto-char (1+ (point)))
+		      (forward-line 1)
 		      (skip-chars-forward "> \t")
 		      (> (current-column) indentation)))
 	  (ebrowse-hide-line collapse)
-	  (skip-chars-forward "^\r\n")
-	  (goto-char (1+ (point))))))))
+	  (forward-line 1))))))
 
 \f
 ;;; Electric tree selection
@@ -2164,7 +2144,7 @@ ebrowse-choose-from-browser-buffers
 ;;;###autoload
 (define-derived-mode ebrowse-member-mode special-mode "Ebrowse-Members"
   "Major mode for Ebrowse member buffers."
-  (mapc 'make-local-variable
+  (mapc #'make-local-variable
 	'(ebrowse--decl-column	        ;display column
 	  ebrowse--n-columns		;number of short columns
 	  ebrowse--column-width	        ;width of columns above
@@ -2587,7 +2567,7 @@ ebrowse-redisplay-member-buffer
   (let ((display-fn (if ebrowse--long-display-flag
 			'ebrowse-draw-member-long-fn
 		      'ebrowse-draw-member-short-fn)))
-    (ebrowse-output
+    (with-silent-modifications
       (erase-buffer)
       ;; Show this class
       (ebrowse-draw-member-buffer-class-line)
@@ -2708,7 +2688,7 @@ ebrowse-draw-member-regexp
 (defun ebrowse-draw-member-long-fn (member-list tree)
   "Display member buffer for MEMBER-LIST in long form.
 TREE is the class tree of MEMBER-LIST."
-  (dolist (member-struc (mapcar 'ebrowse-member-display-p member-list))
+  (dolist (member-struc (mapcar #'ebrowse-member-display-p member-list))
     (when member-struc
       (let ((name (ebrowse-ms-name member-struc))
 	    (start (point)))
@@ -3243,7 +3223,8 @@ ebrowse-tags-read-name
 	(if members
 	    (let* ((name (ebrowse-ignoring-completion-case
 			   (completing-read prompt members nil nil member-name)))
-		   (completion-result (try-completion name members)))
+		   ;; (completion-result (try-completion name members))
+                   )
 	      ;; Cannot rely on `try-completion' returning t for exact
 	      ;; matches!  It returns the name as a string.
 	      (unless (gethash name members)
@@ -3794,14 +3775,13 @@ ebrowse-view/find-position
 	 (find-file (ebrowse-position-file-name position))
 	 (goto-char (ebrowse-position-point position)))
 	(t
-	 (unwind-protect
-	     (progn
-	       (push (function
-		      (lambda ()
-			(goto-char (ebrowse-position-point position))))
-		     view-mode-hook)
-	       (view-file (ebrowse-position-file-name position)))
-	   (pop view-mode-hook)))))
+         (let ((fn (lambda ()
+		     (goto-char (ebrowse-position-point position)))))
+	   (unwind-protect
+               (progn
+	         (add-hook 'view-mode-hook fn)
+	         (view-file (ebrowse-position-file-name position)))
+	     (remove-hook 'view-mode-hook fn))))))
 
 
 (defun ebrowse-push-position (marker info &optional target)
@@ -3904,6 +3884,7 @@ ebrowse-electric-position-mode
   (setq mode-line-buffer-identification "Electric Position Menu")
   (when (memq 'mode-name mode-line-format)
     (setq mode-line-format (copy-sequence mode-line-format))
+    ;; FIXME: Why not set `mode-name' to "Positions"?
     (setcar (memq 'mode-name mode-line-format) "Positions"))
   (set (make-local-variable 'Helper-return-blurb) "return to buffer editing")
   (setq truncate-lines t
@@ -4050,7 +4031,7 @@ ebrowse-save-tree-as
 	  (erase-buffer)
 	  (setf (ebrowse-hs-member-table header) nil)
 	  (insert (prin1-to-string header) " ")
-	  (mapc 'ebrowse-save-class tree)
+	  (mapc #'ebrowse-save-class tree)
 	  (write-file file-name)
 	  (message "Tree written to file `%s'" file-name))
       (kill-buffer temp-buffer)
@@ -4065,7 +4046,7 @@ ebrowse-save-class
   (insert "[ebrowse-ts ")
   (prin1 (ebrowse-ts-class class))	;class name
   (insert "(")				;list of subclasses
-  (mapc 'ebrowse-save-class (ebrowse-ts-subclasses class))
+  (mapc #'ebrowse-save-class (ebrowse-ts-subclasses class))
   (insert ")")
   (dolist (func ebrowse-member-list-accessors)
     (prin1 (funcall func class))
@@ -4252,12 +4233,12 @@ ebrowse-electric-buffer-list
   (unwind-protect
       (progn
 	(add-hook 'electric-buffer-menu-mode-hook
-		  'ebrowse-hack-electric-buffer-menu)
+		  #'ebrowse-hack-electric-buffer-menu)
 	(add-hook 'electric-buffer-menu-mode-hook
-		  'ebrowse-install-1-to-9-keys)
+		  #'ebrowse-install-1-to-9-keys)
 	(call-interactively 'electric-buffer-list))
     (remove-hook 'electric-buffer-menu-mode-hook
-		 'ebrowse-hack-electric-buffer-menu)))
+		 #'ebrowse-hack-electric-buffer-menu)))
 
 \f
 ;;; Mouse support
@@ -4400,8 +4381,7 @@ ebrowse-mouse-1-in-tree-buffer
     (pcase (event-click-count event)
       (2 (pcase property
 	   ('class-name
-	    (let ((collapsed (save-excursion (skip-chars-forward "^\r\n")
-					     (looking-at "\r"))))
+	    (let ((collapsed (ebrowse--hidden-p (line-end-position))))
 	      (ebrowse-collapse-fn (not collapsed))))
 	   ('mark
 	    (ebrowse-toggle-mark-at-point 1)))))))
@@ -4411,9 +4391,7 @@ ebrowse-mouse-1-in-tree-buffer
 (provide 'ebrowse)
 
 ;; Local variables:
-;; eval:(put 'ebrowse-output 'lisp-indent-hook 0)
 ;; eval:(put 'ebrowse-ignoring-completion-case 'lisp-indent-hook 0)
-;; eval:(put 'ebrowse-save-selective 'lisp-indent-hook 0)
 ;; eval:(put 'ebrowse-for-all-trees 'lisp-indent-hook 1)
 ;; End:
 

Re: Eradicating selective-display == t (was: [PATCH 2/3] lisp/progmodes/etags.el don't (forward-char) as it's overriden next line)

[Konstantin Kharlamov]

В Пн, мар 18, 2019 at 12:14 ДП (AM), Stefan Monnier 
<monnier@iro.umontreal.ca> написал:
>>  AFAICT within Emacs's `master` it was only used in Dired and
>>  Ebrowse, so I just pushed a change to get rid of it in Dired.
> 
> I have a patch for Ebrowse as well, but I can't test it, cause I have 
> no
> C++ code at hand to play with.
> 
> Can someone help me test the patch below?

I can test, but only if you tell how. It's first time I hear of 
ebrowse, and from looking at manual and ebrowse-* commands it have vast 
functional, I don't know what to look at.

Re: Eradicating selective-display == t

[Stefan Monnier]

>>>  AFAICT within Emacs's `master` it was only used in Dired and
>>>  Ebrowse, so I just pushed a change to get rid of it in Dired.
>>
>> I have a patch for Ebrowse as well, but I can't test it, cause I have no
>> C++ code at hand to play with.
>>
>> Can someone help me test the patch below?
>
> I can test, but only if you tell how. It's first time I hear of ebrowse, and
> from looking at manual and ebrowse-* commands it have vast functional,
> I don't know what to look at.

I think the only things that need testing are the hide/unhide commands.


        Stefan

Re: [PATCH 2/3] lisp/progmodes/etags.el don't (forward-char) as it's overriden next line

[Eli Zaretskii]

> Date: Sun, 17 Mar 2019 23:44:28 +0300
> From: Konstantin Kharlamov <hi-angel@yandex.ru>
> Cc: emacs-devel@gnu.org
> 
> > But that's incorrect: a tag in a tags table always begins at the
> > beginning of a line, so it must be a search anchored at the beginning
> > of the line.  And in selective-display mode ^M counts as the beginning
> > of a line.
> 
> Great, now that we established that part, to reply your question why we 
> don't care about the beginnging of the line let me quote my other mail:
> 
>  > The pattern that this functions searches for determines the tag 
> uniquely. But here's a catch: no programming language creates distinc 
> entities (ones that end up in the tag), based only on trailing space. 
> I.e. "foo()" and " foo() " always refer to the same thing.

I'm not talking about whitespace.  I'm talking about a tags table file
that names a symbol 'foobar', say.  If you search for a tag "bar" and
do not anchor the search at the beginning of a line, you will decide
that "bar" is present on the "foobar" line, although it really isn't.
Right?

If we want to solve a problem with variable leading whitespace, we
need a more sophisticated solution that just dropping the anchor.

Or did I miss something?

Re: Eradicating selective-display == t

[Konstantin Kharlamov]

On Пн, Mar 18, 2019 at 04:12:09, Stefan Monnier 
<monnier@iro.umontreal.ca> wrote:
>>>>   AFAICT within Emacs's `master` it was only used in Dired and
>>>>   Ebrowse, so I just pushed a change to get rid of it in Dired.
>>> 
>>>  I have a patch for Ebrowse as well, but I can't test it, cause I 
>>> have no
>>>  C++ code at hand to play with.
>>> 
>>>  Can someone help me test the patch below?
>> 
>>  I can test, but only if you tell how. It's first time I hear of 
>> ebrowse, and
>>  from looking at manual and ebrowse-* commands it have vast 
>> functional,
>>  I don't know what to look at.
> 
> I think the only things that need testing are the hide/unhide 
> commands.

Ok, so, with the diff applied, ebrowse-collapse-branch and 
ebrowse-expand-all works fine for me.

Re: [PATCH 2/3] lisp/progmodes/etags.el don't (forward-char) as it's overriden next line

[Konstantin Kharlamov]

On Пн, Mar 18, 2019 at 06:34:00, Eli Zaretskii <eliz@gnu.org> wrote:
>>  Date: Sun, 17 Mar 2019 23:44:28 +0300
>>  From: Konstantin Kharlamov <hi-angel@yandex.ru>
>>  Cc: emacs-devel@gnu.org
>> 
>>  > But that's incorrect: a tag in a tags table always begins at the
>>  > beginning of a line, so it must be a search anchored at the 
>> beginning
>>  > of the line.  And in selective-display mode ^M counts as the 
>> beginning
>>  > of a line.
>> 
>>  Great, now that we established that part, to reply your question 
>> why we
>>  don't care about the beginnging of the line let me quote my other 
>> mail:
>> 
>>   > The pattern that this functions searches for determines the tag
>>  uniquely. But here's a catch: no programming language creates 
>> distinc
>>  entities (ones that end up in the tag), based only on trailing 
>> space.
>>  I.e. "foo()" and " foo() " always refer to the same thing.
> 
> I'm not talking about whitespace.  I'm talking about a tags table file
> that names a symbol 'foobar', say.  If you search for a tag "bar" and
> do not anchor the search at the beginning of a line, you will decide
> that "bar" is present on the "foobar" line, although it really isn't.
> Right?

Not exactly. Here's an alternative bad situation: let's say you do 
anchor the text, and you search for tag 'foo'. And… you still match 
'foobar'!

Actually, it's interesting, that such name clashes are possible right 
now: let's say we have:

	#define FOO
	#define FOO_BAR

If source code is intact, you should get FOO. But if code changed, then 
emacs tries to find where did it go, and may as well stumble upon 
FOO_BAR.

So, I suggest an improvement to my patch: how about we

1. anchor the regexp to the end of the line also
2. replace trailing space with "any whitespace" regex '\s-*'

?

Re: [PATCH 2/3] lisp/progmodes/etags.el don't (forward-char) as it's overriden next line

[Konstantin Kharlamov]

On Пн, Mar 18, 2019 at 12:43:16, Konstantin Kharlamov 
<hi-angel@yandex.ru> wrote:
> 
> 
> On Пн, Mar 18, 2019 at 06:34:00, Eli Zaretskii <eliz@gnu.org> wrote:
>>>  Date: Sun, 17 Mar 2019 23:44:28 +0300
>>>  From: Konstantin Kharlamov <hi-angel@yandex.ru>
>>>  Cc: emacs-devel@gnu.org
>>> 
>>>  > But that's incorrect: a tag in a tags table always begins at the
>>>  > beginning of a line, so it must be a search anchored at the 
>>> \x7f\x7fbeginning
>>>  > of the line.  And in selective-display mode ^M counts as the 
>>> \x7f\x7fbeginning
>>>  > of a line.
>>> 
>>>  Great, now that we established that part, to reply your question 
>>> \x7f\x7fwhy we
>>>  don't care about the beginnging of the line let me quote my other 
>>> \x7f\x7fmail:
>>> 
>>>   > The pattern that this functions searches for determines the tag
>>>  uniquely. But here's a catch: no programming language creates 
>>> \x7f\x7fdistinc
>>>  entities (ones that end up in the tag), based only on trailing 
>>> \x7f\x7fspace.
>>>  I.e. "foo()" and " foo() " always refer to the same thing.
>> 
>> I'm not talking about whitespace.  I'm talking about a tags table 
>> file
>> that names a symbol 'foobar', say.  If you search for a tag "bar" and
>> do not anchor the search at the beginning of a line, you will decide
>> that "bar" is present on the "foobar" line, although it really isn't.
>> Right?
> 
> Not exactly. Here's an alternative bad situation: let's say you do 
> anchor the text, and you search for tag 'foo'. And… you still match 
> 'foobar'!
> 
> Actually, it's interesting, that such name clashes are possible right 
> now: let's say we have:
> 
> 	#define FOO
> 	#define FOO_BAR
> 
> If source code is intact, you should get FOO. But if code changed, 
> then emacs tries to find where did it go, and may as well stumble 
> upon FOO_BAR.
> 
> So, I suggest an improvement to my patch: how about we
> 
> 1. anchor the regexp to the end of the line also
> 2. replace trailing space with "any whitespace" regex '\s-*'
> 
> ?

To be clear: I suggest that we keep anchoring to the beginning of a 
line in place, add anchoring to the end of line, and replace trailing 
whitespace with "any whitespace" regex.

Re: Eradicating selective-display == t

[Stefan Monnier]

> Ok, so, with the diff applied, ebrowse-collapse-branch and
> ebrowse-expand-all works fine for me.

Great, thanks!


        Stefan

Re: [PATCH 2/3] lisp/progmodes/etags.el don't (forward-char) as it's overriden next line

[Eli Zaretskii]

> Date: Mon, 18 Mar 2019 12:43:16 +0300
> From: Konstantin Kharlamov <hi-angel@yandex.ru>
> Cc: emacs-devel@gnu.org
> 
> > I'm not talking about whitespace.  I'm talking about a tags table file
> > that names a symbol 'foobar', say.  If you search for a tag "bar" and
> > do not anchor the search at the beginning of a line, you will decide
> > that "bar" is present on the "foobar" line, although it really isn't.
> > Right?
> 
> Not exactly. Here's an alternative bad situation: let's say you do 
> anchor the text, and you search for tag 'foo'. And… you still match 
> 'foobar'!

That's expected, and how etags.el should work: it finds tags that
_begin_ with the specified text.  Partial matches are filtered out by
higher-level routines, if needed.

But with your proposed change the match will be in the middle of a
symbol as well, something the callers don't expect.

> 	#define FOO
> 	#define FOO_BAR
> 
> If source code is intact, you should get FOO. But if code changed, then 
> emacs tries to find where did it go, and may as well stumble upon 
> FOO_BAR.

That's okay, that's how this low-level stuff is supposed to work.

> So, I suggest an improvement to my patch: how about we
> 
> 1. anchor the regexp to the end of the line also
> 2. replace trailing space with "any whitespace" regex '\s-*'
> 
> ?

etags.el is used by/supports many major modes, and in general I don't
think we want to assume that whitespace in a tag is insignificant,
certainly not as a global change in behavior.

So I would actually suggest to make one step back and describe in more
detail the actual problem you had with the current code.  The anjuta
issue to which you pointed doesn't have enough details, like why the
change in leading whitespace was deemed a problem, and so I cannot
make up my mind what is the actual problem and why it should be fixed
in etags.el.

Can you provide those additional details?

TIA

Re: [PATCH 2/3] lisp/progmodes/etags.el don't (forward-char) as it's overriden next line

[Konstantin Kharlamov]

В Пн, мар 18, 2019 at 8:00 ПП (PM), Eli Zaretskii 
<eliz@gnu.org> написал:
>>  Date: Mon, 18 Mar 2019 12:43:16 +0300
>>  From: Konstantin Kharlamov <hi-angel@yandex.ru>
>>  Cc: emacs-devel@gnu.org
>> 
>>  > I'm not talking about whitespace.  I'm talking about a tags table 
>> file
>>  > that names a symbol 'foobar', say.  If you search for a tag "bar" 
>> and
>>  > do not anchor the search at the beginning of a line, you will 
>> decide
>>  > that "bar" is present on the "foobar" line, although it really 
>> isn't.
>>  > Right?
>> 
>>  Not exactly. Here's an alternative bad situation: let's say you do
>>  anchor the text, and you search for tag 'foo'. And… you still 
>> match
>>  'foobar'!
> 
> That's expected, and how etags.el should work: it finds tags that
> _begin_ with the specified text.  Partial matches are filtered out by
> higher-level routines, if needed.
> 
> But with your proposed change the match will be in the middle of a
> symbol as well, something the callers don't expect.
> 
>>  	#define FOO
>>  	#define FOO_BAR
>> 
>>  If source code is intact, you should get FOO. But if code changed, 
>> then
>>  emacs tries to find where did it go, and may as well stumble upon
>>  FOO_BAR.
> 
> That's okay, that's how this low-level stuff is supposed to work.
> 
>>  So, I suggest an improvement to my patch: how about we
>> 
>>  1. anchor the regexp to the end of the line also
>>  2. replace trailing space with "any whitespace" regex '\s-*'
>> 
>>  ?
> 
> etags.el is used by/supports many major modes, and in general I don't
> think we want to assume that whitespace in a tag is insignificant,
> certainly not as a global change in behavior.
> 
> So I would actually suggest to make one step back and describe in more
> detail the actual problem you had with the current code.  The anjuta
> issue to which you pointed doesn't have enough details, like why the
> change in leading whitespace was deemed a problem, and so I cannot
> make up my mind what is the actual problem and why it should be fixed
> in etags.el.
> 
> Can you provide those additional details?

I'm not sure what additional details you want. I found a usecase where 
heuristic for finding a tag can be improved. I can make even stronger 
statement: all whitespace within a tag is insignificant, not only the 
trailing part.

Let's make it the other way around: please, provide any language 
example, where having the trailing space may be significant for 
unambigiously determining the tag.

Re: [PATCH 2/3] lisp/progmodes/etags.el don't (forward-char) as it's overriden next line

[Konstantin Kharlamov]

В Пн, мар 18, 2019 at 10:15 ПП (PM), Konstantin Kharlamov 
<hi-angel@yandex.ru> написал:
> 
> 
> В Пн, мар 18, 2019 at 8:00 ПП (PM), Eli Zaretskii 
> <eliz@gnu.org> написал:
>>>  Date: Mon, 18 Mar 2019 12:43:16 +0300
>>>  From: Konstantin Kharlamov <hi-angel@yandex.ru>
>>>  Cc: emacs-devel@gnu.org
>>> 
>>>  > I'm not talking about whitespace.  I'm talking about a tags 
>>> table \x7f\x7ffile
>>>  > that names a symbol 'foobar', say.  If you search for a tag 
>>> "bar" \x7f\x7fand
>>>  > do not anchor the search at the beginning of a line, you will 
>>> \x7f\x7fdecide
>>>  > that "bar" is present on the "foobar" line, although it really 
>>> \x7f\x7fisn't.
>>>  > Right?
>>> 
>>>  Not exactly. Here's an alternative bad situation: let's say you do
>>>  anchor the text, and you search for tag 'foo'. And… you still 
>>> \x7f\x7fmatch
>>>  'foobar'!
>> 
>> That's expected, and how etags.el should work: it finds tags that
>> _begin_ with the specified text.  Partial matches are filtered out by
>> higher-level routines, if needed.
>> 
>> But with your proposed change the match will be in the middle of a
>> symbol as well, something the callers don't expect.
>> 
>>>  	#define FOO
>>>  	#define FOO_BAR
>>> 
>>>  If source code is intact, you should get FOO. But if code changed, 
>>> \x7f\x7fthen
>>>  emacs tries to find where did it go, and may as well stumble upon
>>>  FOO_BAR.
>> 
>> That's okay, that's how this low-level stuff is supposed to work.
>> 
>>>  So, I suggest an improvement to my patch: how about we
>>> 
>>>  1. anchor the regexp to the end of the line also
>>>  2. replace trailing space with "any whitespace" regex '\s-*'
>>> 
>>>  ?
>> 
>> etags.el is used by/supports many major modes, and in general I don't
>> think we want to assume that whitespace in a tag is insignificant,
>> certainly not as a global change in behavior.
>> 
>> So I would actually suggest to make one step back and describe in 
>> more
>> detail the actual problem you had with the current code.  The anjuta
>> issue to which you pointed doesn't have enough details, like why the
>> change in leading whitespace was deemed a problem, and so I cannot
>> make up my mind what is the actual problem and why it should be fixed
>> in etags.el.
>> 
>> Can you provide those additional details?
> 
> I'm not sure what additional details you want. I found a usecase 
> where heuristic for finding a tag can be improved. I can make even 
> stronger statement: all whitespace within a tag is insignificant, not 
> only the trailing part.

…as long of course as word and symbols boundaries preserved. Just to 
make it more clear.

> Let's make it the other way around: please, provide any language 
> example, where having the trailing space may be significant for 
> unambigiously determining the tag.

Re: [PATCH 2/3] lisp/progmodes/etags.el don't (forward-char) as it's overriden next line

[Eli Zaretskii]

> Date: Mon, 18 Mar 2019 22:15:27 +0300
> From: Konstantin Kharlamov <hi-angel@yandex.ru>
> Cc: emacs-devel@gnu.org
> 
> > Can you provide those additional details?
> 
> I'm not sure what additional details you want.

OK, let me ask more specific questions:

  1. Why was the amount of whitespace in the tags table different from
     that in the source file?
  2. What command(s) was/were used to produce the tags table?
  3. Why are the issue and you talking about "trailing whitespace",
     while the difference between Expected and Actual in the issue is
     in leading whitespace, not trailing whitespace?

> Let's make it the other way around: please, provide any language 
> example, where having the trailing space may be significant for 
> unambigiously determining the tag.

etags.el doesn't understand any languages, that is the job of the
program that produces the tags table.  etags.el just follows the rules
for the format of tags in the tags table.  I don't know about trailing
whitespace, but tags for HTML files surely include embedded
whitespace, for example.

So once again, I suggest to focus on the particular problem you had,
because I'd like to find a solution for that without making
backward-incompatible changes in behavior of etags.el.

Thanks.

Re: [PATCH 2/3] lisp/progmodes/etags.el don't (forward-char) as it's overriden next line

[Konstantin Kharlamov]

В Пн, мар 18, 2019 at 11:16 ПП (PM), Eli Zaretskii 
<eliz@gnu.org> написал:
>>  Date: Mon, 18 Mar 2019 22:15:27 +0300
>>  From: Konstantin Kharlamov <hi-angel@yandex.ru>
>>  Cc: emacs-devel@gnu.org
>> 
>>  > Can you provide those additional details?
>> 
>>  I'm not sure what additional details you want.
> 
> OK, let me ask more specific questions:
> 
>   1. Why was the amount of whitespace in the tags table different from
>      that in the source file?

It's a bug in anjuta-tags

>   2. What command(s) was/were used to produce the tags table?

anjuta-tags -e file.vala

>   3. Why are the issue and you talking about "trailing whitespace",
>      while the difference between Expected and Actual in the issue is
>      in leading whitespace, not trailing whitespace?

Sorry for confusion here, I was imagining: given a string, whitespace 
may "trail" on both sides of it. But I'm not a native speaker.

>>  Let's make it the other way around: please, provide any language
>>  example, where having the trailing space may be significant for
>>  unambigiously determining the tag.
> 
> etags.el doesn't understand any languages, that is the job of the
> program that produces the tags table.  etags.el just follows the rules
> for the format of tags in the tags table.  I don't know about trailing
> whitespace, but tags for HTML files surely include embedded
> whitespace, for example.
> 
> So once again, I suggest to focus on the particular problem you had,
> because I'd like to find a solution for that without making
> backward-incompatible changes in behavior of etags.el.

Okay, whatever. I get your position. Though I disagree, but we clearly 
have different priorities here: while I'm focusing on a Emacs user, 
you're focusing on technical correctness. I doubt this discussion can 
move anywhere further, let's leave it at it.

Can my 1-st patch from the series be merged though?

Thanks.

Re: [PATCH 2/3] lisp/progmodes/etags.el don't (forward-char) as it's overriden next line

[Konstantin Kharlamov]

В Вт, мар 19, 2019 at 12:45 ДП (AM), Konstantin Kharlamov 
<hi-angel@yandex.ru> написал:
> 
> 
> В Пн, мар 18, 2019 at 11:16 ПП (PM), Eli Zaretskii 
> <eliz@gnu.org> написал:
>>>  Date: Mon, 18 Mar 2019 22:15:27 +0300
>>>  From: Konstantin Kharlamov <hi-angel@yandex.ru>
>>>  Cc: emacs-devel@gnu.org
>>> 
>>>  > Can you provide those additional details?
>>> 
>>>  I'm not sure what additional details you want.
>> 
>> OK, let me ask more specific questions:
>> 
>>   1. Why was the amount of whitespace in the tags table different 
>> from
>>      that in the source file?
> 
> It's a bug in anjuta-tags
> 
>>   2. What command(s) was/were used to produce the tags table?
> 
> anjuta-tags -e file.vala
> 
>>   3. Why are the issue and you talking about "trailing whitespace",
>>      while the difference between Expected and Actual in the issue is
>>      in leading whitespace, not trailing whitespace?
> 
> Sorry for confusion here, I was imagining: given a string, whitespace 
> may "trail" on both sides of it. But I'm not a native speaker.
> 
>>>  Let's make it the other way around: please, provide any language
>>>  example, where having the trailing space may be significant for
>>>  unambigiously determining the tag.
>> 
>> etags.el doesn't understand any languages, that is the job of the
>> program that produces the tags table.  etags.el just follows the 
>> rules
>> for the format of tags in the tags table.  I don't know about 
>> trailing
>> whitespace, but tags for HTML files surely include embedded
>> whitespace, for example.
>> 
>> So once again, I suggest to focus on the particular problem you had,
>> because I'd like to find a solution for that without making
>> backward-incompatible changes in behavior of etags.el.
> 
> Okay, whatever. I get your position. Though I disagree, but we 
> clearly have different priorities here: while I'm focusing on a Emacs 
> user, you're focusing on technical correctness. I doubt this 
> discussion can move anywhere further, let's leave it at it.
> 
> Can my 1-st patch from the series be merged though?

Oh, wait, I noticed despite going to a tag I'm getting "Rerun etags" 
message. Something is wrong, let me investigate.

Re: [PATCH 2/3] lisp/progmodes/etags.el don't (forward-char) as it's overriden next line

[Konstantin Kharlamov]

В Вт, мар 19, 2019 at 2:13 ДП (AM), Konstantin Kharlamov 
<hi-angel@yandex.ru> написал:
> 
> 
> В Вт, мар 19, 2019 at 12:45 ДП (AM), Konstantin Kharlamov 
> <hi-angel@yandex.ru> написал:
>> 
>> 
>> В Пн, мар 18, 2019 at 11:16 ПП (PM), Eli Zaretskii 
>> \x7f<eliz@gnu.org> написал:
>>>>  Date: Mon, 18 Mar 2019 22:15:27 +0300
>>>>  From: Konstantin Kharlamov <hi-angel@yandex.ru>
>>>>  Cc: emacs-devel@gnu.org
>>>> 
>>>>  > Can you provide those additional details?
>>>> 
>>>>  I'm not sure what additional details you want.
>>> 
>>> OK, let me ask more specific questions:
>>> 
>>>   1. Why was the amount of whitespace in the tags table different 
>>> \x7f\x7ffrom
>>>      that in the source file?
>> 
>> It's a bug in anjuta-tags
>> 
>>>   2. What command(s) was/were used to produce the tags table?
>> 
>> anjuta-tags -e file.vala
>> 
>>>   3. Why are the issue and you talking about "trailing whitespace",
>>>      while the difference between Expected and Actual in the issue 
>>> is
>>>      in leading whitespace, not trailing whitespace?
>> 
>> Sorry for confusion here, I was imagining: given a string, 
>> whitespace \x7fmay "trail" on both sides of it. But I'm not a native 
>> speaker.
>> 
>>>>  Let's make it the other way around: please, provide any language
>>>>  example, where having the trailing space may be significant for
>>>>  unambigiously determining the tag.
>>> 
>>> etags.el doesn't understand any languages, that is the job of the
>>> program that produces the tags table.  etags.el just follows the 
>>> \x7f\x7frules
>>> for the format of tags in the tags table.  I don't know about 
>>> \x7f\x7ftrailing
>>> whitespace, but tags for HTML files surely include embedded
>>> whitespace, for example.
>>> 
>>> So once again, I suggest to focus on the particular problem you had,
>>> because I'd like to find a solution for that without making
>>> backward-incompatible changes in behavior of etags.el.
>> 
>> Okay, whatever. I get your position. Though I disagree, but we 
>> \x7fclearly have different priorities here: while I'm focusing on a 
>> Emacs \x7fuser, you're focusing on technical correctness. I doubt this 
>> \x7fdiscussion can move anywhere further, let's leave it at it.
>> 
>> Can my 1-st patch from the series be merged though?
> 
> Oh, wait, I noticed despite going to a tag I'm getting "Rerun etags" 
> message. Something is wrong, let me investigate.

Oh, I found my bug, that sucks, don't know how I missed that, I've used 
that code for a while. I need to return t from the cycle. Basically, I 
need either "break", or recursion. The first isn't supported in Lisp, 
and the tail-recursion optimization is not supported in Emacs. So 
there's actually no way to improve this code.

The patch can be dropped too.

Re: [PATCH 2/3] lisp/progmodes/etags.el don't (forward-char) as it's overriden next line

[Stefan Monnier]

> Oh, I found my bug, that sucks, don't know how I missed that, I've used that
> code for a while. I need to return t from the cycle. Basically, I need
> either "break", or recursion.

There's catch&throw which is a superset of what you can get with `break`.


        Stefan

Re: [PATCH 2/3] lisp/progmodes/etags.el don't (forward-char) as it's overriden next line

[Eli Zaretskii]

> Date: Tue, 19 Mar 2019 00:45:10 +0300
> From: Konstantin Kharlamov <hi-angel@yandex.ru>
> Cc: emacs-devel@gnu.org
> 
> >   1. Why was the amount of whitespace in the tags table different from
> >      that in the source file?
> 
> It's a bug in anjuta-tags

So if/when that bug is fixed (maybe it's fixed already), there will be
no reason to make changes in etags.el?

> >   3. Why are the issue and you talking about "trailing whitespace",
> >      while the difference between Expected and Actual in the issue is
> >      in leading whitespace, not trailing whitespace?
> 
> Sorry for confusion here, I was imagining: given a string, whitespace 
> may "trail" on both sides of it. But I'm not a native speaker.

OK, no problem.  I guessed something like that.

> Okay, whatever. I get your position. Though I disagree, but we clearly 
> have different priorities here: while I'm focusing on a Emacs user, 
> you're focusing on technical correctness.

It's not just technical correctness.  etags.el is used for a very
large number of different "languages" (some of them are not
programming languages at all, like HTML), so any syntactic change that
might be a no-brainer for programming languages could break some other
"language" we currently support.

> Can my 1-st patch from the series be merged though?

I will review that soon, please wait for a little bit longer.  I'm
somewhat hard-pressed for free time lately.  (Or maybe someone else
here could volunteer to review that sooner?)

[PATCH v2] lisp/progmodes/etags.el clean up code by removing a temporary

[Konstantin Kharlamov]

---

v2: I fixed a bug that I didn't return t from "while". The code does not
look as short now as I'd hoped for; it's a little less of the original
size. But I think it's still might be useful, because it refactors the
code to have the relevant functional closer, and removes a variable from
function's visibility scope.

 lisp/progmodes/etags.el | 27 +++++++++++++--------------
 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/lisp/progmodes/etags.el b/lisp/progmodes/etags.el
index c2715be5370..0014c10eb81 100644
--- a/lisp/progmodes/etags.el
+++ b/lisp/progmodes/etags.el
@@ -1351,7 +1351,7 @@ etags-goto-tag-location
 hits the start of file."
   (let ((startpos (cdr (cdr tag-info)))
 	(line (car (cdr tag-info)))
-	offset found pat)
+	offset pat)
     (if (eq (car tag-info) t)
 	;; Direct file tag.
 	(cond (line (progn (goto-char (point-min))
@@ -1363,7 +1363,6 @@ etags-goto-tag-location
       ;; since just going around the loop once probably
       ;; costs about as much as searching 2000 chars.
       (setq offset 1000
-	    found nil
 	    pat (concat (if (eq selective-display t)
 			    "\\(^\\|\^m\\)" "^")
 			(regexp-quote (car tag-info))))
@@ -1385,19 +1384,19 @@ etags-goto-tag-location
 				    (point)))))
       (or startpos
 	  (setq startpos (point-min)))
-      ;; First see if the tag is right at the specified location.
       (goto-char startpos)
-      (setq found (looking-at pat))
-      (while (and (not found)
-		  (progn
-		    (goto-char (- startpos offset))
-		    (not (bobp))))
-	(setq found
-	      (re-search-forward pat (+ startpos offset) t)
-	      offset (* 3 offset)))	; expand search window
-      (or found
-	  (re-search-forward pat nil t)
-	  (user-error "Rerun etags: `%s' not found in %s"
+      (or (looking-at pat) ; Is tag at the specified location?
+          (catch 'found
+            (while (progn
+                     (goto-char (- startpos offset))
+                     (when (bobp)
+                       (throw 'found nil))
+                     (when (re-search-forward pat (+ startpos offset) t)
+                       (throw 'found t))
+                     t)
+              (setq offset (* 3 offset)))) ; expand search window
+          (re-search-forward pat nil t)
+          (user-error "Rerun etags: `%s' not found in %s"
                       pat buffer-file-name)))
     ;; Position point at the right place
     ;; if the search string matched an extra Ctrl-m at the beginning.
-- 
2.21.0