bug#14655: 24.3; (looking-at comment-start-skip) in newcomment.el

bug#14655: 24.3; (looking-at comment-start-skip) in newcomment.el

[Thorsten Jolitz]

This bug report will be sent to the Bug-GNU-Emacs mailing list
and the GNU bug tracker at debbugs.gnu.org.  Please check that
the From: line contains a valid email address.  After a delay of up
to one day, you should receive an acknowledgment at that address.

Please write in English if possible, as the Emacs maintainers
usually do not have translators for other languages.

Please describe exactly what actions triggered the bug, and
the precise symptoms of the bug.  If you can, give a recipe
starting from `emacs -Q':

1. emacs -Q
2. C-x C-f sql-test.sql

-- * data load
-- ** data for table1
drop table1;

3. M-: (goto-char (point-min))

4. M-: (comment-search-forward 15 t)

=> 

Debugger entered--Lisp error: (wrong-type-argument stringp nil)
  looking-at(nil)
  comment-search-forward(15 t)
  eval((comment-search-forward 15 t) nil)
  eval-expression((comment-search-forward 15 t) nil)
  call-interactively(eval-expression nil nil)

because in sql-mode:

5. C-h v comment-start-skip

comment-start-skip is a variable defined in `newcomment.el'.
Its value is nil

Thus, newcomment.el should not call `looking-at' on a variable that
might be nil, because it requires a string-argument.  

If Emacs crashed, and you have the Emacs process in the gdb debugger,
please include the output from the following gdb commands:
    `bt full' and `xbacktrace'.
For information about debugging Emacs, please read the file
/usr/share/emacs/24.3/etc/DEBUG.


In GNU Emacs 24.3.1 (x86_64-unknown-linux-gnu, GTK+ Version 3.8.1)
 of 2013-04-29 on eric
Windowing system distributor `The X.Org Foundation', version 11.0.11401000
Configured using:
 `configure '--prefix=/usr' '--sysconfdir=/etc' '--libexecdir=/usr/lib'
 '--localstatedir=/var' '--with-x-toolkit=gtk3' '--with-xft'
 'CFLAGS=-march=x86-64 -mtune=generic -O2 -pipe -fstack-protector
 --param=ssp-buffer-size=4'
 'LDFLAGS=-Wl,-O1,--sort-common,--as-needed,-z,relro'
 'CPPFLAGS=-D_FORTIFY_SOURCE=2''

Important settings:
  value of $LC_COLLATE: C
  value of $LANG: de_DE.utf8
  locale-coding-system: utf-8-unix
  default enable-multibyte-characters: t

Major mode: Debugger

Minor modes in effect:
  tooltip-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
  buffer-read-only: t
  line-number-mode: t
  transient-mark-mode: t

Recent input:
C-x C-f j u n <tab> s q l - <tab> RET C-n C-n C-n C-SPC 
C-n C-n C-n C-d C-w C-w C-g C-x k RET C-x C-f j u n 
<tab> s q l - <tab> RET M-: ( c o m m e n t - s e a 
r c h - f o r w a r d SPC 1 5 SPC t ) RET M-x e m a 
c s - <tab> C-g M-x b u g - <tab> <tab> <backspace> 
<backspace> <backspace> <backspace> <backspace> <backspace> 
<backspace> <backspace> <backspace> <backspace> <backspace> 
<backspace> <backspace> <backspace> r e p <tab> o <tab> 
r <tab> RET

Recent messages:
Loading vc-git...done
Mark set
call-interactively: End of buffer
...junk/sql-test-junk.sql locked by tj@hostname (pid 927): (s, q, p, ?)? 
Please type q, s, or p; or ? for help
Quit
Entering debugger...
Making completion list...
Quit
Making completion list... [3 times]

Load-path shadows:
/usr/share/emacs/24.3/lisp/org/.dir-locals hides /usr/share/emacs/24.3/lisp/gnus/.dir-locals

Features:
(shadow sort gnus-util mail-extr emacsbug message format-spec rfc822 mml
mml-sec mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev
gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums mm-util
mail-prsvr mail-utils help-mode debug vc-git sql easymenu thingatpt
comint ansi-color ring time-date tooltip ediff-hook vc-hooks
lisp-float-type mwheel x-win x-dnd tool-bar dnd fontset image regexp-opt
fringe tabulated-list newcomment lisp-mode register page menu-bar
rfn-eshadow timer select scroll-bar mouse jit-lock font-lock syntax
facemenu font-core frame cham georgian utf-8-lang misc-lang vietnamese
tibetan thai tai-viet lao korean japanese hebrew greek romanian slovak
czech european ethiopic indian cyrillic chinese case-table epa-hook
jka-cmpr-hook help simple abbrev minibuffer loaddefs button faces
cus-face macroexp files text-properties overlay sha1 md5 base64 format
env code-pages mule custom widget hashtable-print-readable backquote
make-network-process dbusbind dynamic-setting system-font-setting
font-render-setting move-toolbar gtk x-toolkit x multi-tty emacs)

-- 
cheers,
Thorsten

bug#14655: 24.3; (looking-at comment-start-skip) in newcomment.el

[Glenn Morris]

Thorsten Jolitz wrote:

> 4. M-: (comment-search-forward 15 t)

From the commentary of newcomment.el:

;; Prior to calling any `comment-*' function, you should ensure that
;; `comment-normalize-vars' is first called to set up the appropriate
;; variables; except for the `comment-*' commands, which call
;; `comment-normalize-vars' automatically as a subroutine.

bug#14655: 24.3; (looking-at comment-start-skip) in newcomment.el

[Drew Adams]

> From the commentary of newcomment.el:
> 
> ;; Prior to calling any `comment-*' function, you should ensure that
> ;; `comment-normalize-vars' is first called to set up the appropriate
> ;; variables; except for the `comment-*' commands, which call
> ;; `comment-normalize-vars' automatically as a subroutine.

Yes.  I'm not sure what the "solution" is, but this seems to be a
common gotcha.  Yes, someone coding with functions from the library
should presumably read the file Commentary.  Still, we can perhaps do
better, to try to get this info across.

Mentioning it in doc string might help.  But perhaps there is a better
idea.

bug#14655: 24.3; (looking-at comment-start-skip) in newcomment.el

[Thorsten Jolitz]

Drew Adams <drew.adams@oracle.com> writes:

>> From the commentary of newcomment.el:
>> 
>> ;; Prior to calling any `comment-*' function, you should ensure that
>> ;; `comment-normalize-vars' is first called to set up the appropriate
>> ;; variables; except for the `comment-*' commands, which call
>> ;; `comment-normalize-vars' automatically as a subroutine.
>
> Yes.  I'm not sure what the "solution" is, but this seems to be a
> common gotcha.  Yes, someone coding with functions from the library
> should presumably read the file Commentary.  Still, we can perhaps do
> better, to try to get this info across.
>
> Mentioning it in doc string might help.  But perhaps there is a better
> idea.

I find it a bit strange that the library itself does not check the
variables before acting on them. Would something like this be too much
effort or too costly?

,-----------------------------------
| (if comment-start-skip
|    (looking-at comment-start-skip)
|   (comment-normalize-vars)
|   (looking-at comment-start-skip))
`-----------------------------------

Why pass the burden to the users, that then frequently pass it back to
the maintainers because they did not read the docstring. 

-- 
cheers,
Thorsten

bug#14655: 24.3; (looking-at comment-start-skip) in newcomment.el

[Andreas Röhler]

Am 18.06.2013 20:37, schrieb Thorsten Jolitz:
> Drew Adams <drew.adams@oracle.com> writes:
[ ... ]
> I find it a bit strange that the library itself does not check the
> variables before acting on them.

[ ... ]

Indeed. IMHO the whole normalize-vars thing deserves a re-consideration, i.e. simplification.

Andreas