From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Reuben Thomas Newsgroups: gmane.emacs.bugs Subject: bug#25082: [PATCH] Add support to emacsclient for command-lline options in ALTERNATE_EDITOR/--alternate-editor Date: Tue, 29 Aug 2017 16:49:23 +0100 Message-ID: References: <83val93vrk.fsf@gnu.org> <83val62xcb.fsf@gnu.org> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: multipart/alternative; boundary="001a113ce08cf3acde0557e65aac" X-Trace: blaine.gmane.org 1504021832 1416 195.159.176.226 (29 Aug 2017 15:50:32 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Tue, 29 Aug 2017 15:50:32 +0000 (UTC) Cc: 25082@debbugs.gnu.org To: Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Tue Aug 29 17:50:24 2017 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1dmimH-0007fu-Ra for geb-bug-gnu-emacs@m.gmane.org; Tue, 29 Aug 2017 17:50:10 +0200 Original-Received: from localhost ([::1]:45611 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dmimO-0005T6-IO for geb-bug-gnu-emacs@m.gmane.org; Tue, 29 Aug 2017 11:50:16 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:53670) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dmimF-0005QO-SB for bug-gnu-emacs@gnu.org; Tue, 29 Aug 2017 11:50:09 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dmimB-0008Jc-TU for bug-gnu-emacs@gnu.org; Tue, 29 Aug 2017 11:50:07 -0400 Original-Received: from debbugs.gnu.org ([208.118.235.43]:52195) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1dmimB-0008JC-Nf for bug-gnu-emacs@gnu.org; Tue, 29 Aug 2017 11:50:03 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1dmimA-0000zj-7P for bug-gnu-emacs@gnu.org; Tue, 29 Aug 2017 11:50:03 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Reuben Thomas Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Tue, 29 Aug 2017 15:50:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 25082 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 25082-submit@debbugs.gnu.org id=B25082.15040217723771 (code B ref 25082); Tue, 29 Aug 2017 15:50:02 +0000 Original-Received: (at 25082) by debbugs.gnu.org; 29 Aug 2017 15:49:32 +0000 Original-Received: from localhost ([127.0.0.1]:60873 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1dmilg-0000yk-0F for submit@debbugs.gnu.org; Tue, 29 Aug 2017 11:49:32 -0400 Original-Received: from mail-oi0-f52.google.com ([209.85.218.52]:36397) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1dmild-0000yX-KV for 25082@debbugs.gnu.org; Tue, 29 Aug 2017 11:49:31 -0400 Original-Received: by mail-oi0-f52.google.com with SMTP id t75so30694869oie.3 for <25082@debbugs.gnu.org>; Tue, 29 Aug 2017 08:49:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sc3d.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=F+mEQVCMdN0j1kWbdI26/Gejre9ZtAZTBZABFL0doqo=; b=dUa0OJKuEHJa49gn56HwCYUsQMmemabyTDXoj3JIg2uy0QVt6Larl9TSc9y5Fe8ep4 HNf/JPlIPOWkvhxSV4FJTxrY7RxFJi1udCenfu4vZrIcQZC02dCyV32mRqWootozXW4D dPTiV0A30lzQ8OPKQKgVgRlPHlDGZRqN2qqhQ= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=F+mEQVCMdN0j1kWbdI26/Gejre9ZtAZTBZABFL0doqo=; b=BkBBhlf6fuenteA/NnVuvjFej7rSU1nLZrjo1TZUy+fK9bwAxjIgjbP/Vyt0vdZ2Ka qce5yPXMU+X5xft4VjpnivA2jfDYXj0j5by2+7iEsBRtPq2CRClqEJWout4ZBx+xkvoS vuMpfW75tg2b0bGGPcmqdyPylq3BXevJ7UwEEz/+CFj9kZJpO8kkzDz6gRbgh1oj+aMp jSlwdR2AN+suU+poW0xaLugdTaDPtcfDJxeSr/NYjDY/l5/aHPSGVAjpi6Jbr96BooJY Q7dhatDt6QHf/vcZQvYFkEd9Y4dakpUCfedp0Sl1SgYStWRJCs3vOqrz3b617TSt/WXT oQ3A== X-Gm-Message-State: AHYfb5iLwZcCCTJy/xoczrJsVbwcTM4PM2UvOGnTl4lIl5iW5g1ATSI4 wXlIy25L4ry/pFLkvYDHk6e+1PTjYgNZ X-Received: by 10.202.179.137 with SMTP id c131mr711701oif.298.1504021763558; Tue, 29 Aug 2017 08:49:23 -0700 (PDT) Original-Received: by 10.157.53.86 with HTTP; Tue, 29 Aug 2017 08:49:23 -0700 (PDT) In-Reply-To: <83val62xcb.fsf@gnu.org> X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 208.118.235.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.org gmane.emacs.bugs:136324 Archived-At: --001a113ce08cf3acde0557e65aac Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On 29 August 2017 at 16:43, Eli Zaretskii wrote: > > From: Reuben Thomas > > Date: Mon, 28 Aug 2017 11:15:55 +0100 > > Cc: Glenn Morris , 25082@debbugs.gnu.org > > > > >> Is this sufficient? > > > > > > It could be, but I'm not sure I understand clearly what is supported > > > and what isn't. Could you please add this information to the NEWS > > > entry and in more detail to the user manual? I think having these > > > details in the manual is important regardless. > > > > I've added detail to NEWS. I am wary of adding more detail to the > > manual, because it could prevent future improvements (for example, > > implementation of quote escaping): we don't want users to rely on the > > lack of quote escaping. > > We don't want them to rely on the lack of the escaping, but we also > want to tell them what is supported and how. > So just to check, you would view the current lack of documentation of how the current variable works as something that should ideally be improved?=E2= =80=8B > > @table @samp > > @item -a @var{command} > > @itemx --alternate-editor=3D@var{command} > > Specify a command to run if @code{emacsclient} fails to contact Emacs. > > This is useful when running @code{emacsclient} in a script. > > > > Note that this does not document the current situation precisely (a > > user could be forgiven for thinking that "emacs -Q -nw" would already > > work. > > Only if we say "shell command", not just "command". > I don't think many readers will be so precise. I wasn't.=E2=80=8B > > +Arguments may be quoted, so that for example an absolute path > > +containing a space may be specified; quote escaping is not supported. > > I would say `quoted "like this"', since otherwise it isn't clear what > kind of quoting is supported. And I think something similar needs to > be said in the manual. > =E2=80=8BOK, I'll do that.=E2=80=8B > + /* Unpack alternate_editor's space-separated tokens into > new_argv. */ > > + for (char *tok =3D s; tok !=3D NULL && *tok !=3D '\0';) > > + { > > + /* Allocate new token. */ > > + ++toks; > > + new_argv =3D xrealloc (new_argv, new_argv_size + toks * size= of > (char *)); > > + > > + /* Skip leading delimiters, and set separator, skipping any > > + opening quote. */ > > + size_t skip =3D strspn (tok, " \""); > > + tok +=3D skip; > > + char sep =3D (skip > 0 && tok[-1] =3D=3D '"') ? '"' : ' '; > > + > > + /* Record start of token. */ > > + new_argv[toks - 1] =3D tok; > > + > > + /* Find end of token and overwrite it with NUL. */ > > + tok =3D strchr (tok, sep); > > + if (tok !=3D NULL) > > + *tok++ =3D '\0'; > > + } > > + > > + /* Append main_argv arguments to new_argv. */ > > + memcpy (&new_argv[toks], main_argv + optind, extra_args_size); > > > > - execvp (alternate_editor, main_argv + i); > > + execvp (*new_argv, new_argv); > > This won't work on Windows, btw, if the arguments include whitespace. > But that can be fixed by followup changes. > How not? That is precisely the case it aims to support.=E2=80=8B That's the= whole point of dealing with quotes! (Previous versions of the patch didn't support quoting, and so didn't support spaces in arguments; this version has a passing test to show that spaces in quoted arguments work.) --=20 https://rrt.sc3d.org --001a113ce08cf3acde0557e65aac Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
On 2= 9 August 2017 at 16:43, Eli Zaretskii <eliz@gnu.org> wrote:
> From: Reuben Thomas <rrt@sc3d.org>
> Date: Mon, 28 Aug 2017 11:15:55 +0100
> Cc: Glenn Morris <rgm@gnu.org>= ;, 25082@debbugs.gnu.org
>
> >> Is this sufficient?
> >
> > It could be, but I'm not sure I understand clearly what is su= pported
> > and what isn't.=C2=A0 Could you please add this information t= o the NEWS
> > entry and in more detail to the user manual?=C2=A0 I think having= these
> > details in the manual is important regardless.
>
> I've added detail to NEWS. I am wary of adding more detail to the<= br> > manual, because it could prevent future improvements (for example,
> implementation of quote escaping): we don't want users to rely on = the
> lack of quote escaping.

We don't want them to rely on the lack of the escaping, but we a= lso
want to tell them what is supported and how.

So just to che= ck, you would view the current lack of documentation of how the current var= iable works as something that should ideally be improved?=E2=80=8B
=C2=A0
> @table @samp
> @item -a @var{command}
> @itemx --alternate-editor=3D@var{command}
> Specify a command to run if @code{emacsclient} fails to contact Emacs.=
> This is useful when running @code{emacsclient} in a script.
>
> Note that this does not document the current situation precisely (a > user could be forgiven for thinking that "emacs -Q -nw" woul= d already
> work.

Only if we say "shell command", not just "command&quo= t;.

I don't think many readers will be so precise. I wa= sn't.=E2=80=8B
=C2=A0
> +Arguments may be quoted, so that for example an absolute path
> +containing a space may be specified; quote escaping is not supported.=

I would say `quoted "like this"', since otherwise it isn'= t clear what
kind of quoting is supported.=C2=A0 And I think something similar needs to<= br> be said in the manual.

=E2=80=8BOK, I'll do that.=E2=80= =8B

> +=C2=A0 =C2=A0 =C2=A0 /* Unpack alternate_editor's space-separated= tokens into new_argv.=C2=A0 */
> +=C2=A0 =C2=A0 =C2=A0 for (char *tok =3D s; tok !=3D NULL && *= tok !=3D '\0';)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 /* Allocate new token.=C2=A0 */ > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ++toks;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 new_argv =3D xrealloc (new_argv, n= ew_argv_size + toks * sizeof (char *));
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 /* Skip leading delimiters, and se= t separator, skipping any
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0opening quote.=C2=A0 = */
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 size_t skip =3D strspn (tok, "= ; \"");
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 tok +=3D skip;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 char sep =3D (skip > 0 &&am= p; tok[-1] =3D=3D '"') ? '"' : ' ';
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 /* Record start of token.=C2=A0 */=
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 new_argv[toks - 1] =3D tok;
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 /* Find end of token and overwrite= it with NUL.=C2=A0 */
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 tok =3D strchr (tok, sep);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (tok !=3D NULL)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 *tok++ =3D '\0'; > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 }
> +
> +=C2=A0 =C2=A0 =C2=A0 /* Append main_argv arguments to new_argv.=C2=A0= */
> +=C2=A0 =C2=A0 =C2=A0 memcpy (&new_argv[toks], main_argv + optind,= extra_args_size);
>
> -=C2=A0 =C2=A0 =C2=A0 execvp (alternate_editor, main_argv + i);
> +=C2=A0 =C2=A0 =C2=A0 execvp (*new_argv, new_argv);

This won't work on Windows, btw, if the arguments include whitespace. But that can be fixed by followup changes.

<= div>
How not? That is= precisely the case it aims to support.=E2=80=8B That's the whole point= of dealing with quotes! (Previous versions of the patch didn't support= quoting, and so didn't support spaces in arguments; this version has a= passing test to show that spaces in quoted arguments work.)

--
--001a113ce08cf3acde0557e65aac--