bug#17862: 24.3; regexp-opt docstring is incorrect

bug#17862: 24.3; regexp-opt docstring is incorrect

[immerrr again]

[-- Attachment #1: Type: text/plain, Size: 17043 bytes --]

Starting from this commit [1] regexp-opt no longer returns an unenclosed
regular expression.

I'd argue that this sole change requires me to special-case (or ..) form
of rx macro for one arguments to avoid the unnecessary shy-grouping
there, but the message of said commit contains no information about the
background of that change, so it's hard to say if its benefits overweigh
the inconveniences.  What's important though is that the docstring was
never updated along and still describes PAREN as accepting nil to avoid
grouping:

    If optional PAREN is non-nil, ensure that the returned regexp
    is enclosed by at least one regexp grouping construct.

Also, the doc omits that PAREN may be a string to use as an open-paren.

1.
http://git.savannah.gnu.org/cgit/emacs.git/commit/lisp/emacs-lisp/regexp-opt.el?id=540849ee60de0ac129ac63e6ffb5f707c648af1b


In GNU Emacs 24.3.1 (x86_64-pc-linux-gnu, GTK+ Version 3.4.2)
 of 2014-02-22 on king, modified by Debian
Windowing system distributor `The X.Org Foundation', version 11.0.11103000
System Description:    Ubuntu 12.04.4 LTS

Configured using:
 `configure '--build' 'x86_64-linux-gnu' '--build' 'x86_64-linux-gnu'
 '--prefix=/usr' '--sharedstatedir=/var/lib' '--libexecdir=/usr/lib'
 '--localstatedir=/var/lib' '--infodir=/usr/share/info'
 '--mandir=/usr/share/man' '--with-pop=yes'
 '--enable-locallisppath=/etc/emacs24:/etc/emacs:/usr/local/share/emacs/24.3/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/24.3/site-lisp:/usr/share/emacs/site-lisp'
 '--with-crt-dir=/usr/lib/x86_64-linux-gnu' '--with-x=yes'
 '--with-x-toolkit=gtk3' '--with-toolkit-scroll-bars'
 'build_alias=x86_64-linux-gnu' 'CFLAGS=-g -O2 -fstack-protector
 --param=ssp-buffer-size=4 -Wformat -Wformat-security
 -Werror=format-security -Wall' 'LDFLAGS=-Wl,-Bsymbolic-functions
 -Wl,-z,relro' 'CPPFLAGS=-D_FORTIFY_SOURCE=2''

Important settings:
  value of $LC_TIME: en_GB.UTF-8
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix
  default enable-multibyte-characters: t

Major mode: Fundamental

Minor modes in effect:
  csv-field-index-mode: t
  ido-vertical-mode: t
  ido-ubiquitous-mode: t
  global-undo-tree-mode: t
  undo-tree-mode: t
  global-page-break-lines-mode: t
  projectile-global-mode: t
  projectile-mode: t
  global-linum-mode: t
  linum-mode: t
  yas-global-mode: t
  yas-minor-mode: t
  show-paren-mode: t
  auto-compile-on-save-mode: t
  tooltip-mode: t
  mouse-wheel-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  size-indication-mode: t
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t

Recent input:
M-x r e p o r t - e m a <tab> <return>

Recent messages:
require 'region-bindings-mode
el-get: Evaluating :after form for package region-bindings-mode
el-get initialized package region-bindings-mode
el-get initialized package yaml-mode
require 'cython-mode
el-get initialized package cython-mode
el-get: evaluating autoload file
Loading ~/.emacs.d/el-get/.loaddefs...done
(lambda (x) ...) quoted with ' rather than with #' [4 times]
For information about GNU Emacs and the GNU system, type C-h C-a.

Load-path shadows:
/home/immerrr/.emacs.d/el-get/highlight-symbol/.dir-locals hides
/home/immerrr/.emacs.d/el-get/el-get/.dir-locals
/home/immerrr/.emacs.d/el-get/highlight-symbol/.dir-locals hides
/home/immerrr/.emacs.d/el-get/flycheck/.dir-locals
/home/immerrr/.emacs.d/el-get/highlight-symbol/.dir-locals hides
/home/immerrr/.emacs.d/el-get/pkg-info/.dir-locals
/home/immerrr/.emacs.d/el-get/highlight-symbol/.dir-locals hides
/home/immerrr/.emacs.d/el-get/org-mode/.dir-locals
/usr/share/emacs/24.3/site-lisp/debian-startup hides
/usr/share/emacs/site-lisp/debian-startup
/usr/share/emacs/24.3/site-lisp/cmake-data/cmake-mode hides
/usr/share/emacs/site-lisp/cmake-mode
/home/immerrr/.emacs.d/el-get/ido-mode-el/ido hides
/usr/share/emacs/24.3/lisp/ido
/usr/share/emacs24/site-lisp/dictionaries-common/flyspell hides
/usr/share/emacs/24.3/lisp/textmodes/flyspell
/usr/share/emacs24/site-lisp/dictionaries-common/ispell hides
/usr/share/emacs/24.3/lisp/textmodes/ispell
/home/immerrr/.emacs.d/el-get/cperl-mode/cperl-mode hides
/usr/share/emacs/24.3/lisp/progmodes/cperl-mode
/home/immerrr/.emacs.d/el-get/org-mode/lisp/org-entities hides
/usr/share/emacs/24.3/lisp/org/org-entities
/home/immerrr/.emacs.d/el-get/org-mode/lisp/ob-io hides
/usr/share/emacs/24.3/lisp/org/ob-io
/home/immerrr/.emacs.d/el-get/org-mode/lisp/ob-css hides
/usr/share/emacs/24.3/lisp/org/ob-css
/home/immerrr/.emacs.d/el-get/org-mode/lisp/ob-sqlite hides
/usr/share/emacs/24.3/lisp/org/ob-sqlite
/home/immerrr/.emacs.d/el-get/org-mode/lisp/ob-lob hides
/usr/share/emacs/24.3/lisp/org/ob-lob
/home/immerrr/.emacs.d/el-get/org-mode/lisp/org-timer hides
/usr/share/emacs/24.3/lisp/org/org-timer
/home/immerrr/.emacs.d/el-get/org-mode/contrib/lisp/org-vm hides
/usr/share/emacs/24.3/lisp/org/org-vm
/home/immerrr/.emacs.d/el-get/org-mode/lisp/org-element hides
/usr/share/emacs/24.3/lisp/org/org-element
/home/immerrr/.emacs.d/el-get/org-mode/lisp/ob-matlab hides
/usr/share/emacs/24.3/lisp/org/ob-matlab
/home/immerrr/.emacs.d/el-get/org-mode/lisp/ob hides
/usr/share/emacs/24.3/lisp/org/ob
/home/immerrr/.emacs.d/el-get/org-mode/contrib/lisp/org-mac-message hides
/usr/share/emacs/24.3/lisp/org/org-mac-message
/home/immerrr/.emacs.d/el-get/org-mode/lisp/ob-mscgen hides
/usr/share/emacs/24.3/lisp/org/ob-mscgen
/home/immerrr/.emacs.d/el-get/org-mode/lisp/org-inlinetask hides
/usr/share/emacs/24.3/lisp/org/org-inlinetask
/home/immerrr/.emacs.d/el-get/org-mode/lisp/org-pcomplete hides
/usr/share/emacs/24.3/lisp/org/org-pcomplete
/home/immerrr/.emacs.d/el-get/org-mode/lisp/org-datetree hides
/usr/share/emacs/24.3/lisp/org/org-datetree
/home/immerrr/.emacs.d/el-get/org-mode/lisp/ob-ruby hides
/usr/share/emacs/24.3/lisp/org/ob-ruby
/home/immerrr/.emacs.d/el-get/org-mode/lisp/org-src hides
/usr/share/emacs/24.3/lisp/org/org-src
/home/immerrr/.emacs.d/el-get/org-mode/lisp/org-plot hides
/usr/share/emacs/24.3/lisp/org/org-plot
/home/immerrr/.emacs.d/el-get/org-mode/lisp/ob-eval hides
/usr/share/emacs/24.3/lisp/org/ob-eval
/home/immerrr/.emacs.d/el-get/org-mode/lisp/ob-ledger hides
/usr/share/emacs/24.3/lisp/org/ob-ledger
/home/immerrr/.emacs.d/el-get/org-mode/lisp/ob-R hides
/usr/share/emacs/24.3/lisp/org/ob-R
/home/immerrr/.emacs.d/el-get/org-mode/lisp/org-archive hides
/usr/share/emacs/24.3/lisp/org/org-archive
/home/immerrr/.emacs.d/el-get/org-mode/lisp/org-docview hides
/usr/share/emacs/24.3/lisp/org/org-docview
/home/immerrr/.emacs.d/el-get/org-mode/lisp/ob-lilypond hides
/usr/share/emacs/24.3/lisp/org/ob-lilypond
/home/immerrr/.emacs.d/el-get/org-mode/lisp/ob-java hides
/usr/share/emacs/24.3/lisp/org/ob-java
/home/immerrr/.emacs.d/el-get/org-mode/lisp/org-w3m hides
/usr/share/emacs/24.3/lisp/org/org-w3m
/home/immerrr/.emacs.d/el-get/org-mode/lisp/org-info hides
/usr/share/emacs/24.3/lisp/org/org-info
/home/immerrr/.emacs.d/el-get/org-mode/lisp/ob-perl hides
/usr/share/emacs/24.3/lisp/org/ob-perl
/home/immerrr/.emacs.d/el-get/org-mode/lisp/ob-latex hides
/usr/share/emacs/24.3/lisp/org/ob-latex
/home/immerrr/.emacs.d/el-get/org-mode/lisp/ob-python hides
/usr/share/emacs/24.3/lisp/org/ob-python
/home/immerrr/.emacs.d/el-get/org-mode/lisp/org-mouse hides
/usr/share/emacs/24.3/lisp/org/org-mouse
/home/immerrr/.emacs.d/el-get/org-mode/lisp/org-ctags hides
/usr/share/emacs/24.3/lisp/org/org-ctags
/home/immerrr/.emacs.d/el-get/org-mode/lisp/org-capture hides
/usr/share/emacs/24.3/lisp/org/org-capture
/home/immerrr/.emacs.d/el-get/org-mode/lisp/org-habit hides
/usr/share/emacs/24.3/lisp/org/org-habit
/home/immerrr/.emacs.d/el-get/org-mode/lisp/ob-plantuml hides
/usr/share/emacs/24.3/lisp/org/ob-plantuml
/home/immerrr/.emacs.d/el-get/org-mode/lisp/ob-keys hides
/usr/share/emacs/24.3/lisp/org/ob-keys
/home/immerrr/.emacs.d/el-get/org-mode/lisp/org-macs hides
/usr/share/emacs/24.3/lisp/org/org-macs
/home/immerrr/.emacs.d/el-get/org-mode/lisp/ob-sass hides
/usr/share/emacs/24.3/lisp/org/ob-sass
/home/immerrr/.emacs.d/el-get/org-mode/lisp/org-protocol hides
/usr/share/emacs/24.3/lisp/org/org-protocol
/home/immerrr/.emacs.d/el-get/org-mode/contrib/lisp/org-mew hides
/usr/share/emacs/24.3/lisp/org/org-mew
/home/immerrr/.emacs.d/el-get/org-mode/lisp/ob-octave hides
/usr/share/emacs/24.3/lisp/org/ob-octave
/home/immerrr/.emacs.d/el-get/org-mode/lisp/ob-js hides
/usr/share/emacs/24.3/lisp/org/ob-js
/home/immerrr/.emacs.d/el-get/org-mode/lisp/org-irc hides
/usr/share/emacs/24.3/lisp/org/org-irc
/home/immerrr/.emacs.d/el-get/org-mode/lisp/ob-haskell hides
/usr/share/emacs/24.3/lisp/org/ob-haskell
/home/immerrr/.emacs.d/el-get/org-mode/lisp/org-table hides
/usr/share/emacs/24.3/lisp/org/org-table
/home/immerrr/.emacs.d/el-get/org-mode/lisp/ob-emacs-lisp hides
/usr/share/emacs/24.3/lisp/org/ob-emacs-lisp
/home/immerrr/.emacs.d/el-get/org-mode/lisp/org-list hides
/usr/share/emacs/24.3/lisp/org/org-list
/home/immerrr/.emacs.d/el-get/org-mode/lisp/ob-gnuplot hides
/usr/share/emacs/24.3/lisp/org/ob-gnuplot
/home/immerrr/.emacs.d/el-get/org-mode/lisp/org-rmail hides
/usr/share/emacs/24.3/lisp/org/org-rmail
/home/immerrr/.emacs.d/el-get/org-mode/lisp/org-bibtex hides
/usr/share/emacs/24.3/lisp/org/org-bibtex
/home/immerrr/.emacs.d/el-get/org-mode/lisp/org-faces hides
/usr/share/emacs/24.3/lisp/org/org-faces
/home/immerrr/.emacs.d/el-get/org-mode/lisp/org-gnus hides
/usr/share/emacs/24.3/lisp/org/org-gnus
/home/immerrr/.emacs.d/el-get/org-mode/lisp/org hides
/usr/share/emacs/24.3/lisp/org/org
/home/immerrr/.emacs.d/el-get/org-mode/lisp/ob-ocaml hides
/usr/share/emacs/24.3/lisp/org/ob-ocaml
/home/immerrr/.emacs.d/el-get/org-mode/lisp/ob-calc hides
/usr/share/emacs/24.3/lisp/org/ob-calc
/home/immerrr/.emacs.d/el-get/org-mode/lisp/ob-C hides
/usr/share/emacs/24.3/lisp/org/ob-C
/home/immerrr/.emacs.d/el-get/org-mode/lisp/org-attach hides
/usr/share/emacs/24.3/lisp/org/org-attach
/home/immerrr/.emacs.d/el-get/org-mode/lisp/ob-clojure hides
/usr/share/emacs/24.3/lisp/org/ob-clojure
/home/immerrr/.emacs.d/el-get/org-mode/lisp/ob-fortran hides
/usr/share/emacs/24.3/lisp/org/ob-fortran
/home/immerrr/.emacs.d/el-get/org-mode/lisp/ob-tangle hides
/usr/share/emacs/24.3/lisp/org/ob-tangle
/home/immerrr/.emacs.d/el-get/org-mode/lisp/org-feed hides
/usr/share/emacs/24.3/lisp/org/org-feed
/home/immerrr/.emacs.d/el-get/org-mode/lisp/org-footnote hides
/usr/share/emacs/24.3/lisp/org/org-footnote
/home/immerrr/.emacs.d/el-get/org-mode/lisp/org-agenda hides
/usr/share/emacs/24.3/lisp/org/org-agenda
/home/immerrr/.emacs.d/el-get/org-mode/lisp/ob-comint hides
/usr/share/emacs/24.3/lisp/org/ob-comint
/home/immerrr/.emacs.d/el-get/org-mode/lisp/org-indent hides
/usr/share/emacs/24.3/lisp/org/org-indent
/home/immerrr/.emacs.d/el-get/org-mode/lisp/ob-sql hides
/usr/share/emacs/24.3/lisp/org/ob-sql
/home/immerrr/.emacs.d/el-get/org-mode/lisp/ob-picolisp hides
/usr/share/emacs/24.3/lisp/org/ob-picolisp
/home/immerrr/.emacs.d/el-get/org-mode/lisp/ob-lisp hides
/usr/share/emacs/24.3/lisp/org/ob-lisp
/home/immerrr/.emacs.d/el-get/org-mode/lisp/ob-ditaa hides
/usr/share/emacs/24.3/lisp/org/ob-ditaa
/home/immerrr/.emacs.d/el-get/org-mode/lisp/ob-scala hides
/usr/share/emacs/24.3/lisp/org/ob-scala
/home/immerrr/.emacs.d/el-get/org-mode/lisp/ob-scheme hides
/usr/share/emacs/24.3/lisp/org/ob-scheme
/home/immerrr/.emacs.d/el-get/org-mode/lisp/ob-ref hides
/usr/share/emacs/24.3/lisp/org/ob-ref
/home/immerrr/.emacs.d/el-get/org-mode/contrib/lisp/org-wl hides
/usr/share/emacs/24.3/lisp/org/org-wl
/home/immerrr/.emacs.d/el-get/org-mode/lisp/ob-shen hides
/usr/share/emacs/24.3/lisp/org/ob-shen
/home/immerrr/.emacs.d/el-get/org-mode/lisp/org-mobile hides
/usr/share/emacs/24.3/lisp/org/org-mobile
/home/immerrr/.emacs.d/el-get/org-mode/lisp/org-mhe hides
/usr/share/emacs/24.3/lisp/org/org-mhe
/home/immerrr/.emacs.d/el-get/org-mode/lisp/org-clock hides
/usr/share/emacs/24.3/lisp/org/org-clock
/home/immerrr/.emacs.d/el-get/org-mode/lisp/org-crypt hides
/usr/share/emacs/24.3/lisp/org/org-crypt
/home/immerrr/.emacs.d/el-get/org-mode/lisp/ob-sh hides
/usr/share/emacs/24.3/lisp/org/ob-sh
/home/immerrr/.emacs.d/el-get/org-mode/lisp/org-eshell hides
/usr/share/emacs/24.3/lisp/org/org-eshell
/home/immerrr/.emacs.d/el-get/org-mode/lisp/org-id hides
/usr/share/emacs/24.3/lisp/org/org-id
/home/immerrr/.emacs.d/el-get/org-mode/lisp/org-colview hides
/usr/share/emacs/24.3/lisp/org/org-colview
/home/immerrr/.emacs.d/el-get/org-mode/lisp/ob-exp hides
/usr/share/emacs/24.3/lisp/org/ob-exp
/home/immerrr/.emacs.d/el-get/org-mode/lisp/ob-org hides
/usr/share/emacs/24.3/lisp/org/ob-org
/home/immerrr/.emacs.d/el-get/org-mode/lisp/org-compat hides
/usr/share/emacs/24.3/lisp/org/org-compat
/home/immerrr/.emacs.d/el-get/org-mode/lisp/org-loaddefs hides
/usr/share/emacs/24.3/lisp/org/org-loaddefs
/home/immerrr/.emacs.d/el-get/org-mode/lisp/ob-asymptote hides
/usr/share/emacs/24.3/lisp/org/ob-asymptote
/home/immerrr/.emacs.d/el-get/org-mode/lisp/ob-screen hides
/usr/share/emacs/24.3/lisp/org/ob-screen
/home/immerrr/.emacs.d/el-get/org-mode/lisp/org-version hides
/usr/share/emacs/24.3/lisp/org/org-version
/home/immerrr/.emacs.d/el-get/org-mode/lisp/ob-table hides
/usr/share/emacs/24.3/lisp/org/ob-table
/home/immerrr/.emacs.d/el-get/org-mode/lisp/ob-maxima hides
/usr/share/emacs/24.3/lisp/org/ob-maxima
/home/immerrr/.emacs.d/el-get/org-mode/lisp/org-install hides
/usr/share/emacs/24.3/lisp/org/org-install
/home/immerrr/.emacs.d/el-get/org-mode/lisp/org-bbdb hides
/usr/share/emacs/24.3/lisp/org/org-bbdb
/home/immerrr/.emacs.d/el-get/org-mode/lisp/ob-awk hides
/usr/share/emacs/24.3/lisp/org/ob-awk
/home/immerrr/.emacs.d/el-get/org-mode/lisp/ob-dot hides
/usr/share/emacs/24.3/lisp/org/ob-dot
/home/immerrr/.emacs.d/el-get/highlight-symbol/.dir-locals hides
/usr/share/emacs/24.3/lisp/gnus/.dir-locals
/home/immerrr/.emacs.d/el-get/package/elpa/cl-lib-0.3/cl-lib hides
/usr/share/emacs/24.3/lisp/emacs-lisp/cl-lib

Features:
(shadow 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 mail-utils
cl-lib-autoloads minimap-autoloads uniquify dired-x iponweb csv-mode
sort mmr-site-local mmr-smartrep highlight-symbol mmr-python-settings
mmr-cc-settings cc-styles cc-align cc-engine cc-vars cc-defs mmr-el-get
ido-vertical-mode ido-ubiquitous cython-mode
python-el-fgallina-expansions python rx region-bindings-mode guide-key
face-remap popwin smartrep undo-tree diff diminish ein ein-loaddefs smex
ido tempo url url-proxy url-privacy url-expand url-methods url-history
url-cookie url-domsuf url-util url-parse auth-source eieio gnus-util
mm-util mail-prsvr password-cache url-vars mailcap xml-parse doxymacs
filladapt goto-chg page-break-lines helm-config helm-aliases deferred
ace-jump-mode expand-region text-mode-expansions er-basic-expansions
expand-region-custom expand-region-core wgrep redshank-loader projectile
pkg-info grep compile comint ansi-color ring dash s multiple-cursors
mc-separate-operations rectangular-region-mode mc-mark-more thingatpt
mc-cycle-cursors mc-edit-lines multiple-cursors-core rect
auto-complete-config auto-complete popup solarized-dark-theme solarized
color-theme wid-edit linum-ex mmr-org-settings yasnippet derived edmacro
kmacro byte-opt warnings per-window-point .loaddefs el-get
el-get-autoloads el-get-list-packages el-get-notify help-mode easymenu
el-get-dependencies el-get-build el-get-status pp el-get-recipes
el-get-byte-compile el-get-methods el-get-fossil el-get-svn
el-get-pacman el-get-github-zip el-get-github-tar el-get-http-zip
el-get-http-tar el-get-hg el-get-go el-get-git-svn el-get-fink
el-get-emacswiki el-get-http el-get-emacsmirror el-get-github el-get-git
el-get-elpa package el-get-darcs el-get-cvs el-get-bzr el-get-brew
el-get-builtin el-get-apt-get el-get-custom el-get-core autoload cl
dired saveplace paren avoid cus-start cus-load mmr-minimal mmr-ediff
windmove auto-compile easy-mmode packed cl-macs gv bytecomp byte-compile
cconv bar-cursor advice help-fns cl-lib advice-preload devhelp 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)

[-- Attachment #2: Type: text/html, Size: 18324 bytes --]

bug#17862: 24.3; regexp-opt docstring is incorrect

[Stefan Monnier]

> I'd argue that this sole change requires me to special-case (or ..) form
> of rx macro for one arguments to avoid the unnecessary shy-grouping
> there, but the message of said commit contains no information about the
> background of that change, so it's hard to say if its benefits overweigh
> the inconveniences.

It makes the semantics of regexp-opt much simpler/cleaner since the
caller doesn't need to worry about the fact that the returned regexp
might need to be surrounded by additional grouping in order for things
like (concat (regexp-opt ..) "[ \t]+" ...) to work as intended.

A few redundant shy-group wrappers are a very small price to pay for
that, I think.  We could arrange to provide some way to avoid this
wrapper, I guess.


        Stefan

bug#17862: 24.3; regexp-opt docstring is incorrect

[Glenn Morris]

immerrr again wrote:

> Also, the doc omits that PAREN may be a string to use as an open-paren.

That's a weird, old, "feature". I wonder what the point of that is?

(regexp-opt '("ab" "ac") "G")   ->    "Ga[bc]\\)"

bug#17862: 24.3; regexp-opt docstring is incorrect

[immerrr again]

On Mon, Jun 30, 2014 at 5:37 PM, Stefan Monnier
<monnier@iro.umontreal.ca> wrote:
>
>
> It makes the semantics of regexp-opt much simpler/cleaner since the
> caller doesn't need to worry about the fact that the returned regexp
> might need to be surrounded by additional grouping in order for things
> like (concat (regexp-opt ..) "[ \t]+" ...) to work as intended.
>
> A few redundant shy-group wrappers are a very small price to pay for
> that, I think.  We could arrange to provide some way to avoid this
> wrapper, I guess.
>
>
>         Stefan

Sure, I'm all for reducing the number ways to shoot yourself in the foot and I
agree that not grouping by default is more error-prone.  I was rather surprised
to find the fix effectively disabling the power-user feature completely instead
of changing the default behaviour (and frustrated a bit about the docstring
becoming explicitly misguiding).

Cheers,
immerrr

PS. My apologies, I've sent this message directly to Stefan, this is
to leave it in
the tracker for reference.

On Mon, Jun 30, 2014 at 5:37 PM, Stefan Monnier
<monnier@iro.umontreal.ca> wrote:
>> I'd argue that this sole change requires me to special-case (or ..) form
>> of rx macro for one arguments to avoid the unnecessary shy-grouping
>> there, but the message of said commit contains no information about the
>> background of that change, so it's hard to say if its benefits overweigh
>> the inconveniences.
>
> It makes the semantics of regexp-opt much simpler/cleaner since the
> caller doesn't need to worry about the fact that the returned regexp
> might need to be surrounded by additional grouping in order for things
> like (concat (regexp-opt ..) "[ \t]+" ...) to work as intended.
>
> A few redundant shy-group wrappers are a very small price to pay for
> that, I think.  We could arrange to provide some way to avoid this
> wrapper, I guess.
>
>
>         Stefan

bug#17862: 24.3; regexp-opt docstring is incorrect

[Andreas Schwab]

Glenn Morris <rgm@gnu.org> writes:

> immerrr again wrote:
>
>> Also, the doc omits that PAREN may be a string to use as an open-paren.
>
> That's a weird, old, "feature". I wonder what the point of that is?
>
> (regexp-opt '("ab" "ac") "G")   ->    "Ga[bc]\\)"

(regexp-opt '("ab" "ac") "\\(?:") => "\\(?:a[bc]\\)"

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

bug#17862: 24.3; regexp-opt docstring is incorrect

[Glenn Morris]

Andreas Schwab wrote:

> (regexp-opt '("ab" "ac") "\\(?:") => "\\(?:a[bc]\\)"

If that is all, it's no longer needed, since that is the default,
as this report says.
I suppose you could use it to add an explicit group number.
Is it worth it?

bug#17862: 24.3; regexp-opt docstring is incorrect

[Stefan Monnier]

>> (regexp-opt '("ab" "ac") "\\(?:") => "\\(?:a[bc]\\)"
> If that is all, it's no longer needed, since that is the default,
> as this report says.
> I suppose you could use it to add an explicit group number.
> Is it worth it?

I don't think so.


        Stefan

bug#17862: 24.3; regexp-opt docstring is incorrect

[immerrr again]

[-- Attachment #1: Type: text/plain, Size: 402 bytes --]

On Tue, Jul 1, 2014 at 7:22 PM, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
>> I suppose you could use it to add an explicit group number.
>> Is it worth it?
>
> I don't think so.
>

Removing this parameter or not is probably a subject of a different
discussion concerning backward compatibility and whatnot. Within the
scope of this bug, I'd like to suggest the following patch to fix the
issue.

[-- Attachment #2: 0001-Fix-regexp-opt-documentation-bug-17862.patch --]
[-- Type: text/x-patch, Size: 4286 bytes --]

From ba401fc2d5ee090ed065e9f0c0d90ef8a3b1fea4 Mon Sep 17 00:00:00 2001
From: immerrr <immerrr@gmail.com>
Date: Sun, 7 Feb 2016 12:46:37 +0300
Subject: [PATCH] Fix regexp-opt documentation (bug #17862)

* lisp/emacs-lisp/regexp-opt.el (regexp-opt): update PAREN doc

* doc/lispref/searching.texi (Regexp Functions): update PAREN doc
---
 doc/lispref/searching.texi    | 33 ++++++++++++++++++++++-----------
 lisp/emacs-lisp/regexp-opt.el | 29 +++++++++++++++++++++--------
 2 files changed, 43 insertions(+), 19 deletions(-)

diff --git a/doc/lispref/searching.texi b/doc/lispref/searching.texi
index 1243d72..e6de768 100644
--- a/doc/lispref/searching.texi
+++ b/doc/lispref/searching.texi
@@ -946,23 +946,34 @@ Regexp Functions
 more efficient, but is almost never worth the effort.}.
 @c E.g., see http://debbugs.gnu.org/2816
 
-If the optional argument @var{paren} is non-@code{nil}, then the
-returned regular expression is always enclosed by at least one
-parentheses-grouping construct.  If @var{paren} is @code{words}, then
-that construct is additionally surrounded by @samp{\<} and @samp{\>};
-alternatively, if @var{paren} is @code{symbols}, then that construct
-is additionally surrounded by @samp{\_<} and @samp{\_>}
-(@code{symbols} is often appropriate when matching
-programming-language keywords and the like).
+The optional argument @var{paren} can be any of the following:
+
+a string
+    the resulting regexp is surrounded by @code{paren} and @samp{\)},
+    e.g.  use @samp{"\\(?1:"} to produce an explicitly numbered group.
+
+@code{words}
+    the resulting regexp is surrounded by @samp{\<\(} and @samp{\)\>}.
+
+@code{symbols}
+    the resulting regexp is surrounded by @samp{\_<\(} and @samp{\)\_>}
+    (this is often appropriate when maching programming-language
+    keywords and the like).
+
+non-@code{nil}
+    the resulting regexp is surrounded by @samp{\(} and @samp{\)}.
+
+Otherwise the resulting regexp is surrounded by @samp{\(?:} and
+@samp{\)}.
 
 This simplified definition of @code{regexp-opt} produces a
 regular expression which is equivalent to the actual value
-(but not as efficient):
+(but typically more efficient):
 
 @example
 (defun regexp-opt (strings &optional paren)
-  (let ((open-paren (if paren "\\(" ""))
-        (close-paren (if paren "\\)" "")))
+  (let ((open-paren (make-open-paren paren))
+        (close-paren (make-close-paren paren)))
     (concat open-paren
             (mapconcat 'regexp-quote strings "\\|")
             close-paren)))
diff --git a/lisp/emacs-lisp/regexp-opt.el b/lisp/emacs-lisp/regexp-opt.el
index b1e132a..6a3912c 100644
--- a/lisp/emacs-lisp/regexp-opt.el
+++ b/lisp/emacs-lisp/regexp-opt.el
@@ -86,18 +86,31 @@
 ;;;###autoload
 (defun regexp-opt (strings &optional paren)
   "Return a regexp to match a string in the list STRINGS.
-Each string should be unique in STRINGS and should not contain any regexps,
-quoted or not.  If optional PAREN is non-nil, ensure that the returned regexp
-is enclosed by at least one regexp grouping construct.
+Each string should be unique in STRINGS and should not contain
+any regexps, quoted or not.  Optional PAREN specifies how the
+returned regexp is surrounded by grouping constructs.
+
 The returned regexp is typically more efficient than the equivalent regexp:
 
- (let ((open (if PAREN \"\\\\(\" \"\")) (close (if PAREN \"\\\\)\" \"\")))
+ (let ((open (make-open-paren PAREN)) (close (make-close-paren PAREN)))
    (concat open (mapconcat \\='regexp-quote STRINGS \"\\\\|\") close))
 
-If PAREN is `words', then the resulting regexp is additionally surrounded
-by \\=\\< and \\>.
-If PAREN is `symbols', then the resulting regexp is additionally surrounded
-by \\=\\_< and \\_>."
+PAREN can be any of the following:
+
+a string
+    the resulting regexp is surrounded by PAREN and \\), e.g.
+    use \"\\\\(?1:\" to produce an explicitly numbered group.
+
+`words'
+    the resulting regexp is surrounded by \\=\\<\\( and \\)\\>.
+
+`symbols'
+    the resulting regexp is surrounded by \\_<\\( and \\)\\_>.
+
+non-nil
+    the resulting regexp is surrounded by \\( and \\).
+
+Otherwise the resulting regexp is surrounded by \\(?: and \\)."
   (save-match-data
     ;; Recurse on the sorted list.
     (let* ((max-lisp-eval-depth 10000)
-- 
2.7.0

bug#17862: 24.3; regexp-opt docstring is incorrect

[npostavs]

immerrr again <immerrr@gmail.com> writes:

> +non-@code{nil}

Should be just non-nil, I believe.

> +    the resulting regexp is surrounded by @samp{\(} and @samp{\)}.
> +
> +Otherwise the resulting regexp is surrounded by @samp{\(?:} and
> +@samp{\)}.

This is not 100% accurate, here is a counterexample:

    (regexp-opt '("a" "b" "c")) ;=> "[abc]"

Not sure how detailed we want to be about this is in the docs though.

>  
>  This simplified definition of @code{regexp-opt} produces a
>  regular expression which is equivalent to the actual value
> -(but not as efficient):
> +(but typically more efficient):

"not as efficient" refers to the output of the simplified definition.

>  
>  @example
>  (defun regexp-opt (strings &optional paren)
> -  (let ((open-paren (if paren "\\(" ""))
> -        (close-paren (if paren "\\)" "")))
> +  (let ((open-paren (make-open-paren paren))
> +        (close-paren (make-close-paren paren)))

I'm not sure that adding these undefined make-{open,close}-paren
functions makes things any clearer.

>  The returned regexp is typically more efficient than the equivalent regexp:
>  
> - (let ((open (if PAREN \"\\\\(\" \"\")) (close (if PAREN \"\\\\)\" \"\")))
> + (let ((open (make-open-paren PAREN)) (close (make-close-paren PAREN)))

Same comment here.

bug#17862: 24.3; regexp-opt docstring is incorrect

[immerrr again]

[-- Attachment #1: Type: text/plain, Size: 2644 bytes --]

Thank you for the review!

Please, see my answers inline and an updated patch attached.

On Fri, Jul 29, 2016 at 4:10 AM,  <npostavs@users.sourceforge.net> wrote:
> immerrr again <immerrr@gmail.com> writes:
>
>> +non-@code{nil}
>
> Should be just non-nil, I believe.
>

I was following the precedent set by other documentation in searching.texi.

>> +    the resulting regexp is surrounded by @samp{\(} and @samp{\)}.
>> +
>> +Otherwise the resulting regexp is surrounded by @samp{\(?:} and
>> +@samp{\)}.
>
> This is not 100% accurate, here is a counterexample:
>
>     (regexp-opt '("a" "b" "c")) ;=> "[abc]"
>
> Not sure how detailed we want to be about this is in the docs though.
>

Agreed, I think reference documentation must cover all cases.  I have updated
it.  The description still seems concise but might have become confusing and
might need an illustrative example like yours.

>>
>>  This simplified definition of @code{regexp-opt} produces a
>>  regular expression which is equivalent to the actual value
>> -(but not as efficient):
>> +(but typically more efficient):
>
> "not as efficient" refers to the output of the simplified definition.
>

Right.  The paragraph referring to "regexp-opt" got me confused, I have
renamed the simplified version "simplified-regexp-opt" for clarity.

>>
>>  @example
>>  (defun regexp-opt (strings &optional paren)
>> -  (let ((open-paren (if paren "\\(" ""))
>> -        (close-paren (if paren "\\)" "")))
>> +  (let ((open-paren (make-open-paren paren))
>> +        (close-paren (make-close-paren paren)))
>
> I'm not sure that adding these undefined make-{open,close}-paren
> functions makes things any clearer.
>

I agree that adding "impenetrable definitions" might be unclear, but I'd argue
that it's still an improvement because the equivalence statement is
plain false now:

- "words" and "symbol" keywords add special backslash characters
around the result

- if PAREN is nil, the result is still grouped (unless the strings can be
  represented as a character set, but that's not handled in the
simplified version)

I doubt we can put all the logic behind PAREN into the simplified version.  I
made an attempt to clarify the "impenetrable definitions" by their name, but if
that's still unacceptable we might want to drop the "is equivalent to" part
which is currently misleading.

>>  The returned regexp is typically more efficient than the equivalent regexp:
>>
>> - (let ((open (if PAREN \"\\\\(\" \"\")) (close (if PAREN \"\\\\)\" \"\")))
>> + (let ((open (make-open-paren PAREN)) (close (make-close-paren PAREN)))
>
> Same comment here.

Same response here :)

Cheers,
immerrr

[-- Attachment #2: 0001-Fix-regexp-opt-documentation-bug-17862.patch --]
[-- Type: text/x-patch, Size: 5045 bytes --]

From 1cf5026b56b6b1b58f8401c7642d06a2809f1e65 Mon Sep 17 00:00:00 2001
From: immerrr <immerrr@gmail.com>
Date: Sun, 7 Feb 2016 12:46:37 +0300
Subject: [PATCH] Fix regexp-opt documentation (bug #17862)

* lisp/emacs-lisp/regexp-opt.el (regexp-opt): update PAREN doc

* doc/lispref/searching.texi (Regexp Functions): update PAREN doc
---
 doc/lispref/searching.texi    | 43 ++++++++++++++++++++++++++++---------------
 lisp/emacs-lisp/regexp-opt.el | 41 +++++++++++++++++++++++++++++++----------
 2 files changed, 59 insertions(+), 25 deletions(-)

diff --git a/doc/lispref/searching.texi b/doc/lispref/searching.texi
index 1243d72..b0ad435 100644
--- a/doc/lispref/searching.texi
+++ b/doc/lispref/searching.texi
@@ -946,23 +946,36 @@ Regexp Functions
 more efficient, but is almost never worth the effort.}.
 @c E.g., see http://debbugs.gnu.org/2816
 
-If the optional argument @var{paren} is non-@code{nil}, then the
-returned regular expression is always enclosed by at least one
-parentheses-grouping construct.  If @var{paren} is @code{words}, then
-that construct is additionally surrounded by @samp{\<} and @samp{\>};
-alternatively, if @var{paren} is @code{symbols}, then that construct
-is additionally surrounded by @samp{\_<} and @samp{\_>}
-(@code{symbols} is often appropriate when matching
-programming-language keywords and the like).
-
-This simplified definition of @code{regexp-opt} produces a
-regular expression which is equivalent to the actual value
-(but not as efficient):
+The optional argument @var{paren} can be any of the following:
+
+a string
+    the resulting regexp is preceded by @var{paren} and followed by
+    @samp{\)}, e.g. use @samp{"\\(?1:"} to produce an explicitly
+    numbered group.
+
+@code{words}
+    the resulting regexp is surrounded by @samp{\<\(} and @samp{\)\>}.
+
+@code{symbols}
+    the resulting regexp is surrounded by @samp{\_<\(} and @samp{\)\_>}
+    (this is often appropriate when maching programming-language
+    keywords and the like).
+
+non-@code{nil}
+    the resulting regexp is surrounded by @samp{\(} and @samp{\)}.
+
+@code{nil}
+    if all @var{strings} are single-character, the resulting regexp is
+    not surrounded, otherwise it is surrounded by @samp{\(?:} and
+    @samp{\)}.
+
+The resulting regexp of @code{regexp-opt} is equivalent to but usually
+more efficient than that of a simplified version:
 
 @example
-(defun regexp-opt (strings &optional paren)
-  (let ((open-paren (if paren "\\(" ""))
-        (close-paren (if paren "\\)" "")))
+(defun simplified-regexp-opt (strings &optional paren)
+  (let ((open-paren (open-paren-logic-as-described-above paren))
+        (close-paren (close-paren-logic-as-described-above paren)))
     (concat open-paren
             (mapconcat 'regexp-quote strings "\\|")
             close-paren)))
diff --git a/lisp/emacs-lisp/regexp-opt.el b/lisp/emacs-lisp/regexp-opt.el
index b1e132a..080aa32 100644
--- a/lisp/emacs-lisp/regexp-opt.el
+++ b/lisp/emacs-lisp/regexp-opt.el
@@ -86,18 +86,39 @@
 ;;;###autoload
 (defun regexp-opt (strings &optional paren)
   "Return a regexp to match a string in the list STRINGS.
-Each string should be unique in STRINGS and should not contain any regexps,
-quoted or not.  If optional PAREN is non-nil, ensure that the returned regexp
-is enclosed by at least one regexp grouping construct.
-The returned regexp is typically more efficient than the equivalent regexp:
+Each string should be unique in STRINGS and should not contain
+any regexps, quoted or not.  Optional PAREN specifies how the
+returned regexp is surrounded by grouping constructs.
 
- (let ((open (if PAREN \"\\\\(\" \"\")) (close (if PAREN \"\\\\)\" \"\")))
-   (concat open (mapconcat \\='regexp-quote STRINGS \"\\\\|\") close))
+The optional argument PAREN can be any of the following:
 
-If PAREN is `words', then the resulting regexp is additionally surrounded
-by \\=\\< and \\>.
-If PAREN is `symbols', then the resulting regexp is additionally surrounded
-by \\=\\_< and \\_>."
+a string
+    the resulting regexp is preceded by PAREN and followed by
+    \\), e.g.  use \"\\\\(?1:\" to produce an explicitly numbered
+    group.
+
+`words'
+    the resulting regexp is surrounded by \\=\\<\\( and \\)\\>.
+
+`symbols'
+    the resulting regexp is surrounded by \\_<\\( and \\)\\_>.
+
+non-nil
+    the resulting regexp is surrounded by \\( and \\).
+
+nil
+    if all STRINGS are single-character, the resulting regexp is
+    not surrounded, otherwise it is surrounded by \\(?: and \\).
+
+The resulting regexp is equivalent to but usually more efficient
+than that of a simplified version:
+
+ (defun simplified-regexp-opt (strings &optional paren)
+   (let ((open-paren (open-paren-logic-as-described-above paren))
+         (close-paren (close-paren-logic-as-described-above paren)))
+     (concat open-paren
+             (mapconcat 'regexp-quote strings \"\\\\|\")
+             close-paren)))"
   (save-match-data
     ;; Recurse on the sorted list.
     (let* ((max-lisp-eval-depth 10000)
-- 
2.9.2

bug#17862: 24.3; regexp-opt docstring is incorrect

[npostavs]

immerrr again <immerrr@gmail.com> writes:
>>> +non-@code{nil}
>>
>> Should be just non-nil, I believe.
>>
>
> I was following the precedent set by other documentation in
> searching.texi.

Oh, hmm, you're right, most uses are non-@code{nil}.  Somehow I had the
impression that nil should be undecorated (maybe I mixed up with
docstring quoting rules).

>
> I agree that adding "impenetrable definitions" might be unclear, but I'd argue
> that it's still an improvement because the equivalence statement is
> plain false now:
>
> - "words" and "symbol" keywords add special backslash characters
> around the result
>
> - if PAREN is nil, the result is still grouped (unless the strings can be
>   represented as a character set, but that's not handled in the
> simplified version)
>
> I doubt we can put all the logic behind PAREN into the simplified version.

Hah, sounds like a challenge :) How about

    (defun simplified-regexp-opt (strings &optional paren)
      (let ((parens (cond ((eq paren 'words)    '("\\<\\(" . "\\)\\>"))
                          ((eq paren 'symbols) '("\\_<\\(" . "\\)\\_>"))
                          ((null paren)          '("\\(?:" . "\\)"))
                          (t                       '("\\(" . "\\)")))))
        (concat (car paren)
                (mapconcat 'regexp-quote strings "\\|")
                (cdr paren))))

> +@code{nil}
> +    if all @var{strings} are single-character, the resulting regexp is
> +    not surrounded, otherwise it is surrounded by @samp{\(?:} and
> +    @samp{\)}.

Zero character strings also:

    (regexp-opt '("a" "")) ;=> "a?"

How about saying "the regexp may be surrounded with \?(: ) to ensure that
it constitutes a single expression (such that appending a postfix
operator like '+' will apply to the whole expression)."

bug#17862: 24.3; regexp-opt docstring is incorrect

[Noam Postavsky]

On Sat, Jul 30, 2016 at 9:28 AM,  <npostavs@users.sourceforge.net> wrote:
> immerrr again <immerrr@gmail.com> writes:
>> I agree that adding "impenetrable definitions" might be unclear, but I'd argue
>> that it's still an improvement because the equivalence statement is
>> plain false now:
>>
>> - "words" and "symbol" keywords add special backslash characters
>> around the result
>>
>> - if PAREN is nil, the result is still grouped (unless the strings can be
>>   represented as a character set, but that's not handled in the
>> simplified version)
>>
>> I doubt we can put all the logic behind PAREN into the simplified version.
>
> Hah, sounds like a challenge :) How about
>
>     (defun simplified-regexp-opt (strings &optional paren)
>       (let ((parens (cond ((eq paren 'words)    '("\\<\\(" . "\\)\\>"))
>                           ((eq paren 'symbols) '("\\_<\\(" . "\\)\\_>"))
>                           ((null paren)          '("\\(?:" . "\\)"))
>                           (t                       '("\\(" . "\\)")))))
>         (concat (car paren)
>                 (mapconcat 'regexp-quote strings "\\|")
>                 (cdr paren))))
>
>> +@code{nil}
>> +    if all @var{strings} are single-character, the resulting regexp is
>> +    not surrounded, otherwise it is surrounded by @samp{\(?:} and
>> +    @samp{\)}.
>
> Zero character strings also:
>
>     (regexp-opt '("a" "")) ;=> "a?"
>
> How about saying "the regexp may be surrounded with \?(: ) to ensure that
> it constitutes a single expression (such that appending a postfix
> operator like '+' will apply to the whole expression)."
>

ping?

bug#17862: 24.3; regexp-opt docstring is incorrect

[immerrr again]

[-- Attachment #1: Type: text/plain, Size: 1341 bytes --]

On Sun, Aug 21, 2016 at 3:47 PM, Noam Postavsky
<npostavs@users.sourceforge.net> wrote:
> On Sat, Jul 30, 2016 at 9:28 AM,  <npostavs@users.sourceforge.net> wrote:
>>
>> <snip>
>>
>> Hah, sounds like a challenge :) How about
>>
>>     (defun simplified-regexp-opt (strings &optional paren)
>>       (let ((parens (cond ((eq paren 'words)    '("\\<\\(" . "\\)\\>"))
>>                           ((eq paren 'symbols) '("\\_<\\(" . "\\)\\_>"))
>>                           ((null paren)          '("\\(?:" . "\\)"))
>>                           (t                       '("\\(" . "\\)")))))
>>         (concat (car paren)
>>                 (mapconcat 'regexp-quote strings "\\|")
>>                 (cdr paren))))
>>
>>> +@code{nil}
>>> +    if all @var{strings} are single-character, the resulting regexp is
>>> +    not surrounded, otherwise it is surrounded by @samp{\(?:} and
>>> +    @samp{\)}.
>>
>> Zero character strings also:
>>
>>     (regexp-opt '("a" "")) ;=> "a?"
>>
>> How about saying "the regexp may be surrounded with \?(: ) to ensure that
>> it constitutes a single expression (such that appending a postfix
>> operator like '+' will apply to the whole expression)."
>>
>
> ping?

Sorry about that, other things took priority.

I liked your ideas, attached is a patch that borrows quite heavily
from them. What do you think?

[-- Attachment #2: 0001-Fix-regexp-opt-documentation-bug-17862.patch --]
[-- Type: text/x-patch, Size: 5650 bytes --]

From 3801c2fb7c75c1d45ea7a126fdfb801ff13de5a4 Mon Sep 17 00:00:00 2001
From: immerrr <immerrr@gmail.com>
Date: Sun, 7 Feb 2016 12:46:37 +0300
Subject: [PATCH] Fix regexp-opt documentation (bug #17862)

* lisp/emacs-lisp/regexp-opt.el (regexp-opt): update PAREN doc

* doc/lispref/searching.texi (Regexp Functions): update PAREN doc
---
 doc/lispref/searching.texi    | 52 ++++++++++++++++++++++++++++---------------
 lisp/emacs-lisp/regexp-opt.el | 46 +++++++++++++++++++++++++++++---------
 2 files changed, 70 insertions(+), 28 deletions(-)

diff --git a/doc/lispref/searching.texi b/doc/lispref/searching.texi
index 1243d72..e42f630 100644
--- a/doc/lispref/searching.texi
+++ b/doc/lispref/searching.texi
@@ -946,26 +946,42 @@ Regexp Functions
 more efficient, but is almost never worth the effort.}.
 @c E.g., see http://debbugs.gnu.org/2816
 
-If the optional argument @var{paren} is non-@code{nil}, then the
-returned regular expression is always enclosed by at least one
-parentheses-grouping construct.  If @var{paren} is @code{words}, then
-that construct is additionally surrounded by @samp{\<} and @samp{\>};
-alternatively, if @var{paren} is @code{symbols}, then that construct
-is additionally surrounded by @samp{\_<} and @samp{\_>}
-(@code{symbols} is often appropriate when matching
-programming-language keywords and the like).
-
-This simplified definition of @code{regexp-opt} produces a
-regular expression which is equivalent to the actual value
-(but not as efficient):
+The optional argument @var{paren} can be any of the following:
+
+a string
+    the resulting regexp is preceded by @var{paren} and followed by
+    @samp{\)}, e.g. use @samp{"\\(?1:"} to produce an explicitly
+    numbered group.
+
+@code{words}
+    the resulting regexp is surrounded by @samp{\<\(} and @samp{\)\>}.
+
+@code{symbols}
+    the resulting regexp is surrounded by @samp{\_<\(} and @samp{\)\_>}
+    (this is often appropriate when maching programming-language
+    keywords and the like).
+
+non-@code{nil}
+    the resulting regexp is surrounded by @samp{\(} and @samp{\)}.
+
+@code{nil}
+    if all @var{strings} are single-character, the resulting regexp is
+    not surrounded, otherwise it is surrounded by @samp{\(?:} and
+    @samp{\)}.
+
+The resulting regexp of @code{regexp-opt} is equivalent to but usually
+more efficient than that of a simplified version:
 
 @example
-(defun regexp-opt (strings &optional paren)
-  (let ((open-paren (if paren "\\(" ""))
-        (close-paren (if paren "\\)" "")))
-    (concat open-paren
-            (mapconcat 'regexp-quote strings "\\|")
-            close-paren)))
+(defun simplified-regexp-opt (strings &optional paren)
+ (let ((parens (cond ((stringp paren)       (cons paren "\\)"))
+                     ((eq paren 'words)    '("\\<\\(" . "\\)\\>"))
+                     ((eq paren 'symbols) '("\\_<\\(" . "\\)\\_>"))
+                     ((null paren)          '("\\(?:" . "\\)"))
+                     (t                       '("\\(" . "\\)")))))
+   (concat (car paren)
+           (mapconcat 'regexp-quote strings "\\|")
+           (cdr paren))))
 @end example
 @end defun
 
diff --git a/lisp/emacs-lisp/regexp-opt.el b/lisp/emacs-lisp/regexp-opt.el
index b1e132a..95c6109 100644
--- a/lisp/emacs-lisp/regexp-opt.el
+++ b/lisp/emacs-lisp/regexp-opt.el
@@ -86,18 +86,44 @@
 ;;;###autoload
 (defun regexp-opt (strings &optional paren)
   "Return a regexp to match a string in the list STRINGS.
-Each string should be unique in STRINGS and should not contain any regexps,
-quoted or not.  If optional PAREN is non-nil, ensure that the returned regexp
-is enclosed by at least one regexp grouping construct.
-The returned regexp is typically more efficient than the equivalent regexp:
+Each string should be unique in STRINGS and should not contain
+any regexps, quoted or not.  Optional PAREN specifies how the
+returned regexp is surrounded by grouping constructs.
 
- (let ((open (if PAREN \"\\\\(\" \"\")) (close (if PAREN \"\\\\)\" \"\")))
-   (concat open (mapconcat \\='regexp-quote STRINGS \"\\\\|\") close))
+The optional argument PAREN can be any of the following:
 
-If PAREN is `words', then the resulting regexp is additionally surrounded
-by \\=\\< and \\>.
-If PAREN is `symbols', then the resulting regexp is additionally surrounded
-by \\=\\_< and \\_>."
+a string
+    the resulting regexp is preceded by PAREN and followed by
+    \\), e.g.  use \"\\\\(?1:\" to produce an explicitly numbered
+    group.
+
+`words'
+    the resulting regexp is surrounded by \\=\\<\\( and \\)\\>.
+
+`symbols'
+    the resulting regexp is surrounded by \\_<\\( and \\)\\_>.
+
+non-nil
+    the resulting regexp is surrounded by \\( and \\).
+
+nil
+    the resulting regexp is surrounded by \\(?: and \\), if it is
+    necessary to ensure that postfix operator appended to it will
+    apply to the whole expression.
+
+The resulting regexp is equivalent to but usually more efficient
+than that of a simplified version:
+
+ (defun simplified-regexp-opt (strings &optional paren)
+   (let ((parens
+          (cond ((stringp paren)       (cons paren \"\\\\)\"))
+                ((eq paren 'words)    '(\"\\\\\\=<\\\\(\" . \"\\\\)\\\\>\"))
+                ((eq paren 'symbols) '(\"\\\\_<\\\\(\" . \"\\\\)\\\\_>\"))
+                ((null paren)          '(\"\\\\(?:\" . \"\\\\)\"))
+                (t                       '(\"\\\\(\" . \"\\\\)\")))))
+     (concat (car paren)
+             (mapconcat 'regexp-quote strings \"\\\\|\")
+             (cdr paren))))"
   (save-match-data
     ;; Recurse on the sorted list.
     (let* ((max-lisp-eval-depth 10000)
-- 
2.9.2

bug#17862: 24.3; regexp-opt docstring is incorrect

[npostavs]

immerrr again <immerrr@gmail.com> writes:

> Sorry about that, other things took priority.

No problem, I thought maybe you had gone on vacation and forgotten all
about this :) It's not really high priority anyway, just that it would
be nice to get it in before the 25.1 release since it's a doc fix.

> +@code{nil}
> +    if all @var{strings} are single-character, the resulting regexp is
> +    not surrounded, otherwise it is surrounded by @samp{\(?:} and
> +    @samp{\)}.

You forgot to update this sentence to be like the one in the docstring.

> +(defun simplified-regexp-opt (strings &optional paren)
> + (let ((parens (cond ((stringp paren)       (cons paren "\\)"))

Oh right, I missed the stringp case in my definition, good catch.

> +nil
> +    the resulting regexp is surrounded by \\(?: and \\), if it is
> +    necessary to ensure that postfix operator appended to it will
                              ^^^
> +    apply to the whole expression.

Missing the word "a" there, I think.  Other than that, looks good to me.
                              

bug#17862: 24.3; regexp-opt docstring is incorrect

[immerrr again]

[-- Attachment #1: Type: text/plain, Size: 366 bytes --]

On Fri, Aug 26, 2016 at 4:08 AM,  <npostavs@users.sourceforge.net> wrote:
> immerrr again <immerrr@gmail.com> writes:
>
>> <snip>
>
> You forgot to update this sentence to be like the one in the docstring.
>
>> <snip>
>
> Missing the word "a" there, I think.  Other than that, looks good to me.
>

Agreed on both issues. Attaching a fixed version.


Cheers,
immerrr

[-- Attachment #2: 0001-Fix-regexp-opt-documentation-bug-17862.patch --]
[-- Type: text/x-patch, Size: 5681 bytes --]

From a2d6af8efa7e57cd256d613262fd1a72bb7c7594 Mon Sep 17 00:00:00 2001
From: immerrr <immerrr@gmail.com>
Date: Sun, 7 Feb 2016 12:46:37 +0300
Subject: [PATCH] Fix regexp-opt documentation (bug #17862)

* lisp/emacs-lisp/regexp-opt.el (regexp-opt): update PAREN doc

* doc/lispref/searching.texi (Regexp Functions): update PAREN doc
---
 doc/lispref/searching.texi    | 52 ++++++++++++++++++++++++++++---------------
 lisp/emacs-lisp/regexp-opt.el | 46 +++++++++++++++++++++++++++++---------
 2 files changed, 70 insertions(+), 28 deletions(-)

diff --git a/doc/lispref/searching.texi b/doc/lispref/searching.texi
index 1243d72..13c7e64 100644
--- a/doc/lispref/searching.texi
+++ b/doc/lispref/searching.texi
@@ -946,26 +946,42 @@ Regexp Functions
 more efficient, but is almost never worth the effort.}.
 @c E.g., see http://debbugs.gnu.org/2816
 
-If the optional argument @var{paren} is non-@code{nil}, then the
-returned regular expression is always enclosed by at least one
-parentheses-grouping construct.  If @var{paren} is @code{words}, then
-that construct is additionally surrounded by @samp{\<} and @samp{\>};
-alternatively, if @var{paren} is @code{symbols}, then that construct
-is additionally surrounded by @samp{\_<} and @samp{\_>}
-(@code{symbols} is often appropriate when matching
-programming-language keywords and the like).
-
-This simplified definition of @code{regexp-opt} produces a
-regular expression which is equivalent to the actual value
-(but not as efficient):
+The optional argument @var{paren} can be any of the following:
+
+a string
+    the resulting regexp is preceded by @var{paren} and followed by
+    @samp{\)}, e.g. use @samp{"\\(?1:"} to produce an explicitly
+    numbered group.
+
+@code{words}
+    the resulting regexp is surrounded by @samp{\<\(} and @samp{\)\>}.
+
+@code{symbols}
+    the resulting regexp is surrounded by @samp{\_<\(} and @samp{\)\_>}
+    (this is often appropriate when maching programming-language
+    keywords and the like).
+
+non-@code{nil}
+    the resulting regexp is surrounded by @samp{\(} and @samp{\)}.
+
+@code{nil}
+    the resulting regexp is surrounded by @samp{\(?:} and @samp{\)},
+    if it is necessary to ensure that a postfix operator appended to
+    it will apply to the whole expression.
+
+The resulting regexp of @code{regexp-opt} is equivalent to but usually
+more efficient than that of a simplified version:
 
 @example
-(defun regexp-opt (strings &optional paren)
-  (let ((open-paren (if paren "\\(" ""))
-        (close-paren (if paren "\\)" "")))
-    (concat open-paren
-            (mapconcat 'regexp-quote strings "\\|")
-            close-paren)))
+(defun simplified-regexp-opt (strings &optional paren)
+ (let ((parens (cond ((stringp paren)       (cons paren "\\)"))
+                     ((eq paren 'words)    '("\\<\\(" . "\\)\\>"))
+                     ((eq paren 'symbols) '("\\_<\\(" . "\\)\\_>"))
+                     ((null paren)          '("\\(?:" . "\\)"))
+                     (t                       '("\\(" . "\\)")))))
+   (concat (car paren)
+           (mapconcat 'regexp-quote strings "\\|")
+           (cdr paren))))
 @end example
 @end defun
 
diff --git a/lisp/emacs-lisp/regexp-opt.el b/lisp/emacs-lisp/regexp-opt.el
index b1e132a..27db477 100644
--- a/lisp/emacs-lisp/regexp-opt.el
+++ b/lisp/emacs-lisp/regexp-opt.el
@@ -86,18 +86,44 @@
 ;;;###autoload
 (defun regexp-opt (strings &optional paren)
   "Return a regexp to match a string in the list STRINGS.
-Each string should be unique in STRINGS and should not contain any regexps,
-quoted or not.  If optional PAREN is non-nil, ensure that the returned regexp
-is enclosed by at least one regexp grouping construct.
-The returned regexp is typically more efficient than the equivalent regexp:
+Each string should be unique in STRINGS and should not contain
+any regexps, quoted or not.  Optional PAREN specifies how the
+returned regexp is surrounded by grouping constructs.
 
- (let ((open (if PAREN \"\\\\(\" \"\")) (close (if PAREN \"\\\\)\" \"\")))
-   (concat open (mapconcat \\='regexp-quote STRINGS \"\\\\|\") close))
+The optional argument PAREN can be any of the following:
 
-If PAREN is `words', then the resulting regexp is additionally surrounded
-by \\=\\< and \\>.
-If PAREN is `symbols', then the resulting regexp is additionally surrounded
-by \\=\\_< and \\_>."
+a string
+    the resulting regexp is preceded by PAREN and followed by
+    \\), e.g.  use \"\\\\(?1:\" to produce an explicitly numbered
+    group.
+
+`words'
+    the resulting regexp is surrounded by \\=\\<\\( and \\)\\>.
+
+`symbols'
+    the resulting regexp is surrounded by \\_<\\( and \\)\\_>.
+
+non-nil
+    the resulting regexp is surrounded by \\( and \\).
+
+nil
+    the resulting regexp is surrounded by \\(?: and \\), if it is
+    necessary to ensure that a postfix operator appended to it will
+    apply to the whole expression.
+
+The resulting regexp is equivalent to but usually more efficient
+than that of a simplified version:
+
+ (defun simplified-regexp-opt (strings &optional paren)
+   (let ((parens
+          (cond ((stringp paren)       (cons paren \"\\\\)\"))
+                ((eq paren 'words)    '(\"\\\\\\=<\\\\(\" . \"\\\\)\\\\>\"))
+                ((eq paren 'symbols) '(\"\\\\_<\\\\(\" . \"\\\\)\\\\_>\"))
+                ((null paren)          '(\"\\\\(?:\" . \"\\\\)\"))
+                (t                       '(\"\\\\(\" . \"\\\\)\")))))
+     (concat (car paren)
+             (mapconcat 'regexp-quote strings \"\\\\|\")
+             (cdr paren))))"
   (save-match-data
     ;; Recurse on the sorted list.
     (let* ((max-lisp-eval-depth 10000)
-- 
2.9.2

bug#17862: 24.3; regexp-opt docstring is incorrect

[npostavs]

immerrr again <immerrr@gmail.com> writes:

> Agreed on both issues. Attaching a fixed version.

Okay, looks ready to merge.  What's your copyright status?

> Subject: [PATCH] Fix regexp-opt documentation (bug #17862)
>
> * lisp/emacs-lisp/regexp-opt.el (regexp-opt): update PAREN doc
>
> * doc/lispref/searching.texi (Regexp Functions): update PAREN doc

Oh, the changelog entry text should be a sentence (capitalized, end with
a period).  And I guess you could combine the 2 changelog entries into
1, i.e.,

* lisp/emacs-lisp/regexp-opt.el (regexp-opt):
* doc/lispref/searching.texi (Regexp Functions): Update PAREN doc.

bug#17862: 24.3; regexp-opt docstring is incorrect

[Eli Zaretskii]

> From: npostavs@users.sourceforge.net
> Date: Thu, 01 Sep 2016 23:06:49 -0400
> Cc: 17862@debbugs.gnu.org, Andreas Schwab <schwab@suse.de>
> 
> immerrr again <immerrr@gmail.com> writes:
> 
> > Agreed on both issues. Attaching a fixed version.
> 
> Okay, looks ready to merge.  What's your copyright status?

His assignment is on file.

bug#17862: 24.3; regexp-opt docstring is incorrect

[immerrr again]

> Oh, the changelog entry text should be a sentence (capitalized, end with
> a period).  And I guess you could combine the 2 changelog entries into
> 1

Attaching the fixed patch.

On Fri, Sep 2, 2016 at 10:03 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>
> His assignment is on file.

My existing assignment is accompanied by a disclaimer from my past
employer. I'm now self-employed, so probably a new disclaimer might be
in order, however AFAICS the long-term contract I'm fulfilling right
now has no clauses concerning the rights to the code I write. I might
need to bring this question to copyright assignment clerk.

bug#17862: 24.3; regexp-opt docstring is incorrect

[immerrr again]

[-- Attachment #1: Type: text/plain, Size: 289 bytes --]

On Fri, Sep 2, 2016 at 3:30 PM, immerrr again <immerrr@gmail.com> wrote:
>> Oh, the changelog entry text should be a sentence (capitalized, end with
>> a period).  And I guess you could combine the 2 changelog entries into
>> 1
>
> Attaching the fixed patch.
>

Now attaching it for good.

[-- Attachment #2: 0001-Fix-regexp-opt-documentation-bug-17862.patch --]
[-- Type: text/x-patch, Size: 5664 bytes --]

From 7e9452a59dec35db9e6ef17efec269f5d4cf3833 Mon Sep 17 00:00:00 2001
From: immerrr <immerrr@gmail.com>
Date: Sun, 7 Feb 2016 12:46:37 +0300
Subject: [PATCH] Fix regexp-opt documentation (bug #17862)

* lisp/emacs-lisp/regexp-opt.el (regexp-opt):
* doc/lispref/searching.texi (Regexp Functions): Update PAREN doc.
---
 doc/lispref/searching.texi    | 52 ++++++++++++++++++++++++++++---------------
 lisp/emacs-lisp/regexp-opt.el | 46 +++++++++++++++++++++++++++++---------
 2 files changed, 70 insertions(+), 28 deletions(-)

diff --git a/doc/lispref/searching.texi b/doc/lispref/searching.texi
index 1243d72..13c7e64 100644
--- a/doc/lispref/searching.texi
+++ b/doc/lispref/searching.texi
@@ -946,26 +946,42 @@ Regexp Functions
 more efficient, but is almost never worth the effort.}.
 @c E.g., see http://debbugs.gnu.org/2816
 
-If the optional argument @var{paren} is non-@code{nil}, then the
-returned regular expression is always enclosed by at least one
-parentheses-grouping construct.  If @var{paren} is @code{words}, then
-that construct is additionally surrounded by @samp{\<} and @samp{\>};
-alternatively, if @var{paren} is @code{symbols}, then that construct
-is additionally surrounded by @samp{\_<} and @samp{\_>}
-(@code{symbols} is often appropriate when matching
-programming-language keywords and the like).
-
-This simplified definition of @code{regexp-opt} produces a
-regular expression which is equivalent to the actual value
-(but not as efficient):
+The optional argument @var{paren} can be any of the following:
+
+a string
+    the resulting regexp is preceded by @var{paren} and followed by
+    @samp{\)}, e.g. use @samp{"\\(?1:"} to produce an explicitly
+    numbered group.
+
+@code{words}
+    the resulting regexp is surrounded by @samp{\<\(} and @samp{\)\>}.
+
+@code{symbols}
+    the resulting regexp is surrounded by @samp{\_<\(} and @samp{\)\_>}
+    (this is often appropriate when maching programming-language
+    keywords and the like).
+
+non-@code{nil}
+    the resulting regexp is surrounded by @samp{\(} and @samp{\)}.
+
+@code{nil}
+    the resulting regexp is surrounded by @samp{\(?:} and @samp{\)},
+    if it is necessary to ensure that a postfix operator appended to
+    it will apply to the whole expression.
+
+The resulting regexp of @code{regexp-opt} is equivalent to but usually
+more efficient than that of a simplified version:
 
 @example
-(defun regexp-opt (strings &optional paren)
-  (let ((open-paren (if paren "\\(" ""))
-        (close-paren (if paren "\\)" "")))
-    (concat open-paren
-            (mapconcat 'regexp-quote strings "\\|")
-            close-paren)))
+(defun simplified-regexp-opt (strings &optional paren)
+ (let ((parens (cond ((stringp paren)       (cons paren "\\)"))
+                     ((eq paren 'words)    '("\\<\\(" . "\\)\\>"))
+                     ((eq paren 'symbols) '("\\_<\\(" . "\\)\\_>"))
+                     ((null paren)          '("\\(?:" . "\\)"))
+                     (t                       '("\\(" . "\\)")))))
+   (concat (car paren)
+           (mapconcat 'regexp-quote strings "\\|")
+           (cdr paren))))
 @end example
 @end defun
 
diff --git a/lisp/emacs-lisp/regexp-opt.el b/lisp/emacs-lisp/regexp-opt.el
index b1e132a..27db477 100644
--- a/lisp/emacs-lisp/regexp-opt.el
+++ b/lisp/emacs-lisp/regexp-opt.el
@@ -86,18 +86,44 @@
 ;;;###autoload
 (defun regexp-opt (strings &optional paren)
   "Return a regexp to match a string in the list STRINGS.
-Each string should be unique in STRINGS and should not contain any regexps,
-quoted or not.  If optional PAREN is non-nil, ensure that the returned regexp
-is enclosed by at least one regexp grouping construct.
-The returned regexp is typically more efficient than the equivalent regexp:
+Each string should be unique in STRINGS and should not contain
+any regexps, quoted or not.  Optional PAREN specifies how the
+returned regexp is surrounded by grouping constructs.
 
- (let ((open (if PAREN \"\\\\(\" \"\")) (close (if PAREN \"\\\\)\" \"\")))
-   (concat open (mapconcat \\='regexp-quote STRINGS \"\\\\|\") close))
+The optional argument PAREN can be any of the following:
 
-If PAREN is `words', then the resulting regexp is additionally surrounded
-by \\=\\< and \\>.
-If PAREN is `symbols', then the resulting regexp is additionally surrounded
-by \\=\\_< and \\_>."
+a string
+    the resulting regexp is preceded by PAREN and followed by
+    \\), e.g.  use \"\\\\(?1:\" to produce an explicitly numbered
+    group.
+
+`words'
+    the resulting regexp is surrounded by \\=\\<\\( and \\)\\>.
+
+`symbols'
+    the resulting regexp is surrounded by \\_<\\( and \\)\\_>.
+
+non-nil
+    the resulting regexp is surrounded by \\( and \\).
+
+nil
+    the resulting regexp is surrounded by \\(?: and \\), if it is
+    necessary to ensure that a postfix operator appended to it will
+    apply to the whole expression.
+
+The resulting regexp is equivalent to but usually more efficient
+than that of a simplified version:
+
+ (defun simplified-regexp-opt (strings &optional paren)
+   (let ((parens
+          (cond ((stringp paren)       (cons paren \"\\\\)\"))
+                ((eq paren 'words)    '(\"\\\\\\=<\\\\(\" . \"\\\\)\\\\>\"))
+                ((eq paren 'symbols) '(\"\\\\_<\\\\(\" . \"\\\\)\\\\_>\"))
+                ((null paren)          '(\"\\\\(?:\" . \"\\\\)\"))
+                (t                       '(\"\\\\(\" . \"\\\\)\")))))
+     (concat (car paren)
+             (mapconcat 'regexp-quote strings \"\\\\|\")
+             (cdr paren))))"
   (save-match-data
     ;; Recurse on the sorted list.
     (let* ((max-lisp-eval-depth 10000)
-- 
2.9.3

bug#17862: 24.3; regexp-opt docstring is incorrect

[Eli Zaretskii]

> From: immerrr again <immerrr@gmail.com>
> Date: Fri, 2 Sep 2016 15:30:34 +0300
> Cc: Noam Postavsky <npostavs@users.sourceforge.net>, 17862@debbugs.gnu.org, 
> 	Andreas Schwab <schwab@suse.de>
> 
> > His assignment is on file.
> 
> My existing assignment is accompanied by a disclaimer from my past
> employer. I'm now self-employed, so probably a new disclaimer might be
> in order

If you are self-employed, then AFAIK your previous employer's
disclaimer is simply not relevant, but your own still stands.  Of
course, IANAL.

> I might need to bring this question to copyright assignment clerk.

That is always a good idea.

Thanks.

bug#17862: 24.3; regexp-opt docstring is incorrect

[Noam Postavsky]

On Fri, Sep 2, 2016 at 9:11 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>>
>> My existing assignment is accompanied by a disclaimer from my past
>> employer. I'm now self-employed, so probably a new disclaimer might be
>> in order
>
> If you are self-employed, then AFAIK your previous employer's
> disclaimer is simply not relevant, but your own still stands.  Of
> course, IANAL.
>
>> I might need to bring this question to copyright assignment clerk.
>
> That is always a good idea.

So is it okay to push this, or should we wait on the copyright assignment clerk?

bug#17862: 24.3; regexp-opt docstring is incorrect

[Eli Zaretskii]

> From: Noam Postavsky <npostavs@users.sourceforge.net>
> Date: Sat, 3 Sep 2016 16:11:30 -0400
> Cc: immerrr again <immerrr@gmail.com>, 17862@debbugs.gnu.org, 
> 	Andreas Schwab <schwab@suse.de>
> 
> On Fri, Sep 2, 2016 at 9:11 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> >>
> >> My existing assignment is accompanied by a disclaimer from my past
> >> employer. I'm now self-employed, so probably a new disclaimer might be
> >> in order
> >
> > If you are self-employed, then AFAIK your previous employer's
> > disclaimer is simply not relevant, but your own still stands.  Of
> > course, IANAL.
> >
> >> I might need to bring this question to copyright assignment clerk.
> >
> > That is always a good idea.
> 
> So is it okay to push this, or should we wait on the copyright assignment clerk?

OK to push, thanks.

bug#17862: 24.3; regexp-opt docstring is incorrect

[npostavs]

tags 17862 fixed
close 17862 25.1
quit

Eli Zaretskii <eliz@gnu.org> writes:

>
> OK to push, thanks.

I've pushed as 56bf7d7e.