all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* bug#35567: 27.0.50; Emacs-Lisp and ElDoc confusion with some strings
@ 2019-05-04 21:14 Mauro Aranda
  2019-08-10 14:00 ` Mauro Aranda
  0 siblings, 1 reply; 4+ messages in thread
From: Mauro Aranda @ 2019-05-04 21:14 UTC (permalink / raw)
  To: 35567


[-- Attachment #1.1: Type: text/plain, Size: 4847 bytes --]

Hello.

I've found some cases where ElDoc displays function information in
the echo area, when it shouldn't.

Try the following, with the attached test.el (no need to eval it):
1) emacs -Q
2) C-x C-f test.el
3) There are two 'and' in the doc string of foo.  Do a search for 'and'
and stop after the first:
C-s and C-f: ElDoc doesn't show a function in the echo area, that's
fine.
Now search for the next 'and':
C-s C-s C-f: ElDoc shows `and: (CONDITIONS...)' in the echo area.  That
shouldn't happen.
4) Also, there's a " in a comment, in the last line.  Search for "here",
just like for "and" before:
C-s here C-f: ElDoc again shows `and: (CONDITIONS...)'.  That again,
shouldn't happen.

In these cases, I see that elisp--fnsym-in-current-sexp doesn't return nil,
as it should when point is inside a string.  So far, I've only found that
a quoted ( confuses it, when the ) is not quoted.  But for the bug to
become visible, the word that follows ")" has to be an interned symbol.

Another example is in the file lisp/files.el.  Put point in the doc string
of
the variable file-name-version-regexp, and see that ElDoc shows
`while: (TEST BODY...)' in the echo area.  I think that happens because
there are " in some comments lines before the doc string.

It seems that the problem is that elisp--beginning-of-sexp calls
forward-sexp, even though its doc string explicitly says that it assumes
point is not in a string or a comment.  Most of the time it still works,
but it fails in some cases, and in those cases, the check for the current
word not being inside a string loses its purpose.

Perhaps using the return value of syntax-ppss before calling
elisp--beginning-of-sexp would be enough to avoid this confusion.

Also, let me ask: was there any discussion about the usefulness of having
ElDoc display information when point is inside a comment?  I ask because
I only see the intention to check when not inside a string.

Best regards,
Mauro.

In GNU Emacs 27.0.50 (build 1, i686-pc-linux-gnu, GTK+ Version 3.18.9)
 of 2019-05-04 built on the-blackbeard
Repository revision: a90ad2297391cf5fe284a4c6e81724360ed018a4
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.11906000
System Description: Ubuntu 16.04.6 LTS

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.

Configured features:
XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND GPM DBUS GSETTINGS GLIB
NOTIFY INOTIFY ACL LIBSELINUX GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF
XFT ZLIB TOOLKIT_SCROLL_BARS GTK3 X11 XDBE XIM THREADS PDUMPER LCMS2 GMP

Important settings:
  value of $LANG: en_US.utf8
  value of $XMODIFIERS:
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny seq byte-opt gv
bytecomp byte-compile cconv dired dired-loaddefs format-spec rfc822 mml
easymenu mml-sec password-cache epa derived epg epg-config gnus-util
rmail rmail-loaddefs text-property-search time-date mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader cl-loaddefs
cl-lib sendmail rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils
elec-pair mule-util tooltip eldoc electric uniquify ediff-hook vc-hooks
lisp-float-type mwheel term/x-win x-win term/common-win x-dnd tool-bar
dnd fontset image regexp-opt fringe tabulated-list replace newcomment
text-mode elisp-mode lisp-mode prog-mode register page menu-bar
rfn-eshadow isearch timer select scroll-bar mouse jit-lock font-lock
syntax facemenu font-core term/tty-colors frame cl-generic cham georgian
utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean
japanese eucjp-ms cp51932 hebrew greek romanian slovak czech european
ethiopic indian cyrillic chinese composite charscript charprop
case-table epa-hook jka-cmpr-hook help simple abbrev obarray minibuffer
cl-preloaded nadvice loaddefs button faces cus-face macroexp files
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget hashtable-print-readable backquote threads dbusbind
inotify lcms2 dynamic-setting system-font-setting font-render-setting
move-toolbar gtk x-toolkit x multi-tty make-network-process emacs)

Memory information:
((conses 8 44043 5027)
 (symbols 24 5870 1)
 (strings 16 14982 1524)
 (string-bytes 1 501645)
 (vectors 8 8861)
 (vector-slots 4 114202 9774)
 (floats 8 17 14)
 (intervals 28 191 4)
 (buffers 564 11)
 (heap 1024 7516 761))

[-- Attachment #1.2: Type: text/html, Size: 5606 bytes --]

[-- Attachment #2: test.el --]
[-- Type: text/x-emacs-lisp, Size: 350 bytes --]

(defun foo ()
  "Doc string of foo.
If you have to quote a parenthesis at the bol, it will be trouble for
elisp-eldoc-documentation-function.
Write (like this) and you will find no problem.  But put
\(a quoted parenthesis) and you'll confuse elisp-eldoc-documentation-function."
  nil)

;; Also, in comments, look what happens when you put a " here.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* bug#35567: 27.0.50; Emacs-Lisp and ElDoc confusion with some strings
  2019-05-04 21:14 bug#35567: 27.0.50; Emacs-Lisp and ElDoc confusion with some strings Mauro Aranda
@ 2019-08-10 14:00 ` Mauro Aranda
  2019-08-10 14:03   ` Mauro Aranda
  0 siblings, 1 reply; 4+ messages in thread
From: Mauro Aranda @ 2019-08-10 14:00 UTC (permalink / raw)
  To: 35567


[-- Attachment #1.1: Type: text/plain, Size: 80 bytes --]

Hello.

I propose the attached patch to fix this problem.

Best regards,
Mauro.

[-- Attachment #1.2: Type: text/html, Size: 174 bytes --]

[-- Attachment #2: 0001-Don-t-display-wrong-ElDoc-information-when-inside-EL.patch --]
[-- Type: text/x-patch, Size: 1742 bytes --]

From bb399d3591d433b88682040f595c82fadf86c978 Mon Sep 17 00:00:00 2001
From: Mauro Aranda <maurooaranda@gmail.com>
Date: Sat, 10 Aug 2019 10:48:24 -0300
Subject: [PATCH] Don't display wrong ElDoc information when inside ELisp
 strings or comments

* lisp/progmodes/elisp-mode.el (elisp--fnsym-in-current-sexp): Since
forward-sexp assumes point is not in a string or comment, avoid
calling it and then checking if point inside a string, since that
sometimes will fail with awkward results.  (Bug#35567)
---
 lisp/progmodes/elisp-mode.el | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/lisp/progmodes/elisp-mode.el b/lisp/progmodes/elisp-mode.el
index 36797fc..f229d07 100644
--- a/lisp/progmodes/elisp-mode.el
+++ b/lisp/progmodes/elisp-mode.el
@@ -1571,14 +1571,12 @@ elisp--docstring-first-line
 ;; Return a list of current function name and argument index.
 (defun elisp--fnsym-in-current-sexp ()
   (save-excursion
-    (let ((argument-index (1- (elisp--beginning-of-sexp))))
-      ;; If we are at the beginning of function name, this will be -1.
-      (when (< argument-index 0)
-	(setq argument-index 0))
-      ;; Don't do anything if current word is inside a string.
-      (if (= (or (char-after (1- (point))) 0) ?\")
-	  nil
-	(list (elisp--current-symbol) argument-index)))))
+    (unless (nth 8 (syntax-ppss))
+      (let ((argument-index (1- (elisp--beginning-of-sexp))))
+        ;; If we are at the beginning of function name, this will be -1.
+        (when (< argument-index 0)
+	  (setq argument-index 0))
+        (list (elisp--current-symbol) argument-index)))))
 
 ;; Move to the beginning of current sexp.  Return the number of nested
 ;; sexp the point was over or after.
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* bug#35567: 27.0.50; Emacs-Lisp and ElDoc confusion with some strings
  2019-08-10 14:00 ` Mauro Aranda
@ 2019-08-10 14:03   ` Mauro Aranda
  2019-08-20 12:42     ` Noam Postavsky
  0 siblings, 1 reply; 4+ messages in thread
From: Mauro Aranda @ 2019-08-10 14:03 UTC (permalink / raw)
  To: 35567


[-- Attachment #1.1: Type: text/plain, Size: 55 bytes --]

Oops, previous patch had a typo in the commit message.

[-- Attachment #1.2: Type: text/html, Size: 87 bytes --]

[-- Attachment #2: 0001-Don-t-display-wrong-ElDoc-information-when-inside-EL.patch --]
[-- Type: text/x-patch, Size: 1745 bytes --]

From b6636d8ac641660c397bc38e191a06e0ddc8b1c7 Mon Sep 17 00:00:00 2001
From: Mauro Aranda <maurooaranda@gmail.com>
Date: Sat, 10 Aug 2019 10:48:24 -0300
Subject: [PATCH] Don't display wrong ElDoc information when inside ELisp
 strings or comments

* lisp/progmodes/elisp-mode.el (elisp--fnsym-in-current-sexp): Since
forward-sexp assumes point is not in a string or comment, avoid
calling it and then checking if point is inside a string, since that
sometimes will fail with awkward results.  (Bug#35567)
---
 lisp/progmodes/elisp-mode.el | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/lisp/progmodes/elisp-mode.el b/lisp/progmodes/elisp-mode.el
index 36797fc..f229d07 100644
--- a/lisp/progmodes/elisp-mode.el
+++ b/lisp/progmodes/elisp-mode.el
@@ -1571,14 +1571,12 @@ elisp--docstring-first-line
 ;; Return a list of current function name and argument index.
 (defun elisp--fnsym-in-current-sexp ()
   (save-excursion
-    (let ((argument-index (1- (elisp--beginning-of-sexp))))
-      ;; If we are at the beginning of function name, this will be -1.
-      (when (< argument-index 0)
-	(setq argument-index 0))
-      ;; Don't do anything if current word is inside a string.
-      (if (= (or (char-after (1- (point))) 0) ?\")
-	  nil
-	(list (elisp--current-symbol) argument-index)))))
+    (unless (nth 8 (syntax-ppss))
+      (let ((argument-index (1- (elisp--beginning-of-sexp))))
+        ;; If we are at the beginning of function name, this will be -1.
+        (when (< argument-index 0)
+	  (setq argument-index 0))
+        (list (elisp--current-symbol) argument-index)))))
 
 ;; Move to the beginning of current sexp.  Return the number of nested
 ;; sexp the point was over or after.
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* bug#35567: 27.0.50; Emacs-Lisp and ElDoc confusion with some strings
  2019-08-10 14:03   ` Mauro Aranda
@ 2019-08-20 12:42     ` Noam Postavsky
  0 siblings, 0 replies; 4+ messages in thread
From: Noam Postavsky @ 2019-08-20 12:42 UTC (permalink / raw)
  To: Mauro Aranda; +Cc: 35567

tags 35567 fixed
close 35567 27.1
quit

> From: Mauro Aranda <maurooaranda@gmail.com>
> Date: Sat, 10 Aug 2019 10:48:24 -0300
> Subject: [PATCH] Don't display wrong ElDoc information when inside ELisp
>  strings or comments

Pushed to master.

4e0c5830da 2019-08-20T08:34:23-04:00 "Don't display wrong ElDoc information when inside ELisp strings or comments"
https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=4e0c5830dab466be2df99b8814b742a662d67ac7






^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2019-08-20 12:42 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-05-04 21:14 bug#35567: 27.0.50; Emacs-Lisp and ElDoc confusion with some strings Mauro Aranda
2019-08-10 14:00 ` Mauro Aranda
2019-08-10 14:03   ` Mauro Aranda
2019-08-20 12:42     ` Noam Postavsky

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.