From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Jay Kamat Newsgroups: gmane.emacs.bugs Subject: bug#28323: Patchset to fix 28323 Date: Tue, 08 May 2018 13:30:46 -0700 Message-ID: <871selgaa1.fsf@gmail.com> References: <8737865dvh.fsf@gmail.com> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-Trace: blaine.gmane.org 1525811360 1952 195.159.176.226 (8 May 2018 20:29:20 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Tue, 8 May 2018 20:29:20 +0000 (UTC) User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) To: 28323@debbugs.gnu.org Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Tue May 08 22:29:15 2018 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 1fG9EY-0000Ol-5Z for geb-bug-gnu-emacs@m.gmane.org; Tue, 08 May 2018 22:29:15 +0200 Original-Received: from localhost ([::1]:53237 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fG9Gf-0003w4-97 for geb-bug-gnu-emacs@m.gmane.org; Tue, 08 May 2018 16:31:25 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:51957) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fG9GL-0003tt-Fu for bug-gnu-emacs@gnu.org; Tue, 08 May 2018 16:31:07 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fG9GI-0003Hd-4d for bug-gnu-emacs@gnu.org; Tue, 08 May 2018 16:31:05 -0400 Original-Received: from debbugs.gnu.org ([208.118.235.43]:46667) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1fG9GH-0003HR-QN for bug-gnu-emacs@gnu.org; Tue, 08 May 2018 16:31:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1fG9GH-0006Pe-J8 for bug-gnu-emacs@gnu.org; Tue, 08 May 2018 16:31:01 -0400 X-Loop: help-debbugs@gnu.org In-Reply-To: <8737865dvh.fsf@gmail.com> Resent-From: Jay Kamat Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Tue, 08 May 2018 20:31:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 28323 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: confirmed Original-Received: via spool by 28323-submit@debbugs.gnu.org id=B28323.152581145624637 (code B ref 28323); Tue, 08 May 2018 20:31:01 +0000 Original-Received: (at 28323) by debbugs.gnu.org; 8 May 2018 20:30:56 +0000 Original-Received: from localhost ([127.0.0.1]:54564 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1fG9GB-0006PJ-R6 for submit@debbugs.gnu.org; Tue, 08 May 2018 16:30:56 -0400 Original-Received: from mail-pf0-f194.google.com ([209.85.192.194]:46455) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1fG9GA-0006P0-BN for 28323@debbugs.gnu.org; Tue, 08 May 2018 16:30:55 -0400 Original-Received: by mail-pf0-f194.google.com with SMTP id p12so24475340pff.13 for <28323@debbugs.gnu.org>; Tue, 08 May 2018 13:30:54 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:subject:date:message-id:user-agent:mime-version; bh=vudi/PBnap4zMOjph6sTdPCr59SxAav03NrV2otxUBQ=; b=pBns8JGdjlQP6C24zHYGaqcJliQgSG/L2zuejDME0djnhZkviX/9aEWjSoAYVyYJXd 77F9kEoL2R20SnIDhAfekGo/Czsci+rTdpszTM/WOZqRnQEN/qJQln+FJBjPBEvTvLVu r95kpmmf/nmMWXBhxi9YVKDqRL4VGV1fkQnPwZjpjUep5XJ7fnbeA4CEOekefvlWca7S GEVSACX0GsM4QYJwHmIWaxUgeseZsv7DqvvjzzSrKQOJ4mo15PCpMlO3HieZ3kB0AaAS 6tSG1jVrx1lCcTt23iydALv2igKsUjMNtG1Xb6riegQwbHDfERlfWInWI2c73M95Q8J0 vh4g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:user-agent :mime-version; bh=vudi/PBnap4zMOjph6sTdPCr59SxAav03NrV2otxUBQ=; b=Z6DW0qNXS0dW4CwFSssMtujZz/IdcgPLzqDKgcGtaoaTaLvnEOJ5gKTKvzqFGJgrIc GnAgU0t+d2mHEh75JBTKbg3nZCsI8WwuJkJ9N295olK2cpzxZsRZ7+Z1GSZ/hW/PINCI mrZBIsQCQZEoFCtPoJEKqif3cfFxFUdNP5roTiEnHlGQz4pKWSCIM4A/XZ+yyESlmpHA yQAyThya1v9AkINLFgP/ZzjXvRC8n31P4n/TEgl7m2wVUk45d7mjmxhUpmAKBjch3xad M1lE9F33nUpvXR9MsA73UMcRIimJa5hITU99e4dQNYwpIaO4nmjE5s2S+dAMPIjkjXCq ALnA== X-Gm-Message-State: ALQs6tAWw0cOiz2DqSnA8RCFU3DGAI+6UVyEjcBoKp865QRaC0WrHIhg cXF5j6IXActYm1aGptNWdK5kEoiI X-Google-Smtp-Source: AB8JxZqW80ZbBa909lCDD8J6LwCDpZMWOBSt11FENJF8GmN7o1O2WQoDVIUq/qWtJAmiCDEAf2RF+w== X-Received: by 2002:a65:6496:: with SMTP id e22-v6mr15868017pgv.386.1525811448288; Tue, 08 May 2018 13:30:48 -0700 (PDT) Original-Received: from laythe (c-24-4-217-5.hsd1.ca.comcast.net. [24.4.217.5]) by smtp.gmail.com with ESMTPSA id b4sm46358553pfa.64.2018.05.08.13.30.47 for <28323@debbugs.gnu.org> (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 08 May 2018 13:30:47 -0700 (PDT) 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:146085 Archived-At: --=-=-= Content-Type: text/plain Hi, This was bugging me for other commands I was running (emerge -uDN world), so I decided to try to write a patch for this. In the process, I found another blocking bug, which would have broken the -u flag entirely. Currently, 'sudo -u root whoami' returns '-u', because of a bug involved with processing leading positional arguments. I fixed the blocking bug first in a separate patch, and then added a new parameter for commands like sudo, which would prefer :raw-positional arguments after the first non flag argument. I'm still not sure if this is the best name for this idea, so if anyone has a better name I'd be happy to change it! this section of eshell seems very important, so I also added some tests, contained in the second patch. Please let me know if anything seems wrong, as I didn't understand the code as well as I would have liked. Commit 1: --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename=0001-esh-opt.el-Fix-improper-parsing-of-first-argument.patch >From 2fd7a43a95ee26b4fab1a0d713dc6d8a994e67bc Mon Sep 17 00:00:00 2001 From: Jay Kamat Date: Tue, 8 May 2018 12:04:00 -0700 Subject: [PATCH 1/2] esh-opt.el: Fix improper parsing of first argument * lisp/eshell/esh-opt.el (eshell--process-args): Refactor usage of args to eshell--args, as we rely on modifications from eshell--process-option and vice versa. These modifications were not being propogated in the (if (= ai 0)) case, since we cannot destructively modify the first element of a list. Examples of broken behavior: sudo -u root whoami ls -I '*.txt' /dev/null --- lisp/eshell/esh-opt.el | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/lisp/eshell/esh-opt.el b/lisp/eshell/esh-opt.el index b802696306..67b7d05985 100644 --- a/lisp/eshell/esh-opt.el +++ b/lisp/eshell/esh-opt.el @@ -246,26 +246,27 @@ eshell--process-args options))) (ai 0) arg (eshell--args args)) - (while (< ai (length args)) - (setq arg (nth ai args)) + (while (< ai (length eshell--args)) + (setq arg (nth ai eshell--args)) (if (not (and (stringp arg) (string-match "^-\\(-\\)?\\(.*\\)" arg))) (setq ai (1+ ai)) (let* ((dash (match-string 1 arg)) (switch (match-string 2 arg))) (if (= ai 0) - (setq args (cdr args)) - (setcdr (nthcdr (1- ai) args) (nthcdr (1+ ai) args))) + (setq eshell--args (cdr eshell--args)) + (setcdr (nthcdr (1- ai) eshell--args) + (nthcdr (1+ ai) eshell--args))) (if dash (if (> (length switch) 0) (eshell--process-option name switch 1 ai options opt-vals) - (setq ai (length args))) + (setq ai (length eshell--args))) (let ((len (length switch)) (index 0)) (while (< index len) (eshell--process-option name (aref switch index) 0 ai options opt-vals) (setq index (1+ index)))))))) - (nconc (mapcar #'cdr opt-vals) args))) + (nconc (mapcar #'cdr opt-vals) eshell--args))) ;;; esh-opt.el ends here -- 2.14.2 --=-=-= Content-Type: text/plain Commit 2: --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename=0002-esh-opt.el-Add-a-raw-positional-argument.patch >From 05c1a597ce302144ea7114a3ed3867fdd4dca950 Mon Sep 17 00:00:00 2001 From: Jay Kamat Date: Tue, 8 May 2018 12:36:36 -0700 Subject: [PATCH 2/2] esh-opt.el: Add a :raw-positional argument * lisp/eshell/esh-opt.el: Add a new :raw-positional argument which ignores dash/switch arguments after the first positional argument. Useful for eshell/sudo, to avoid parsing subcommand switches as switches of the root command. (eshell--process-option): add a new posarg argument which signals that we have found a positional argument already. * test/lisp/eshell/esh-opt-tests.el: Add tests for new and old behavior --- lisp/eshell/em-tramp.el | 1 + lisp/eshell/esh-opt.el | 27 ++++++--- test/lisp/eshell/esh-opt-tests.el | 124 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 145 insertions(+), 7 deletions(-) create mode 100644 test/lisp/eshell/esh-opt-tests.el diff --git a/lisp/eshell/em-tramp.el b/lisp/eshell/em-tramp.el index 004c495490..9057de6da6 100644 --- a/lisp/eshell/em-tramp.el +++ b/lisp/eshell/em-tramp.el @@ -107,6 +107,7 @@ eshell/sudo '((?h "help" nil nil "show this usage screen") (?u "user" t user "execute a command as another USER") :show-usage + :raw-positional :usage "[(-u | --user) USER] COMMAND Execute a COMMAND as the superuser or another USER.") (throw 'eshell-external diff --git a/lisp/eshell/esh-opt.el b/lisp/eshell/esh-opt.el index 67b7d05985..2ca4b03f52 100644 --- a/lisp/eshell/esh-opt.el +++ b/lisp/eshell/esh-opt.el @@ -80,6 +80,10 @@ eshell-eval-using-options If present, do not pass MACRO-ARGS through `eshell-flatten-list' and `eshell-stringify-list'. +:raw-positional + If present, do not parse dash or switch arguments after the first +positional argument. Instead, treat them as positional arguments themselves. + For example, OPTIONS might look like: ((?C nil nil multi-column \"multi-column display\") @@ -203,7 +207,7 @@ eshell--set-option (setq eshell--args (cdr eshell--args))))) (or (nth 2 opt) t))))) -(defun eshell--process-option (name switch kind ai options opt-vals) +(defun eshell--process-option (name switch kind ai posarg options opt-vals) "For NAME, process SWITCH (of type KIND), from args at index AI. The SWITCH will be looked up in the set of OPTIONS. @@ -219,7 +223,10 @@ eshell--process-option (while opts (if (and (listp (car opts)) (nth kind (car opts)) - (equal switch (nth kind (car opts)))) + (equal switch (nth kind (car opts))) + ;; If we want to ignore arguments after a pos one, don't find. + (not (and posarg + (memq ':raw-positional options)))) (progn (eshell--set-option name ai (car opts) options opt-vals) (setq found t opts nil)) @@ -245,27 +252,33 @@ eshell--process-args (list sym))))) options))) (ai 0) arg - (eshell--args args)) + (eshell--args args) + (pos-argument-found nil)) (while (< ai (length eshell--args)) (setq arg (nth ai eshell--args)) (if (not (and (stringp arg) (string-match "^-\\(-\\)?\\(.*\\)" arg))) - (setq ai (1+ ai)) + ;; Positional argument found, skip + (setq ai (1+ ai) + pos-argument-found t) + ;; dash or switch argument found, parse (let* ((dash (match-string 1 arg)) (switch (match-string 2 arg))) (if (= ai 0) (setq eshell--args (cdr eshell--args)) - (setcdr (nthcdr (1- ai) eshell--args) + (setcdr (nthcdr (1- ai) eshell--args) (nthcdr (1+ ai) eshell--args))) (if dash (if (> (length switch) 0) - (eshell--process-option name switch 1 ai options opt-vals) + (eshell--process-option name switch 1 ai pos-argument-found + options opt-vals) (setq ai (length eshell--args))) (let ((len (length switch)) (index 0)) (while (< index len) (eshell--process-option name (aref switch index) - 0 ai options opt-vals) + 0 ai pos-argument-found + options opt-vals) (setq index (1+ index)))))))) (nconc (mapcar #'cdr opt-vals) eshell--args))) diff --git a/test/lisp/eshell/esh-opt-tests.el b/test/lisp/eshell/esh-opt-tests.el new file mode 100644 index 0000000000..b0d59b0edd --- /dev/null +++ b/test/lisp/eshell/esh-opt-tests.el @@ -0,0 +1,124 @@ +;;; tests/esh-opt-tests.el --- esh-opt test suite + +;; Copyright (C) 2018 Free Software Foundation, Inc. + +;; This file is part of GNU Emacs. + +;; GNU Emacs is free software: you can redistribute it and/or modify +;; it under the terms of the GNU General Public License as published by +;; the Free Software Foundation, either version 3 of the License, or +;; (at your option) any later version. + +;; GNU Emacs is distributed in the hope that it will be useful, +;; but WITHOUT ANY WARRANTY; without even the implied warranty of +;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +;; GNU General Public License for more details. + +;; You should have received a copy of the GNU General Public License +;; along with GNU Emacs. If not, see . + +;;; Code: + +(require 'ert) +(require 'esh-opt) + +(ert-deftest esh-opt-process-args-test () + "Unit tests which verify correct behavior of `eshell--process-args'." + (should + (equal '(t) + (eshell--process-args + "sudo" + '("-a") + '((?a "all" nil show-all ""))))) + (should + (equal '(nil) + (eshell--process-args + "sudo" + '("-g") + '((?a "all" nil show-all ""))))) + (should + (equal '("root" "world") + (eshell--process-args + "sudo" + '("-u" "root" "world") + '((?u "user" t user "execute a command as another USER"))))) + (should + (equal '(nil "emerge" "world") + (eshell--process-args + "sudo" + '("emerge" "-uDN" "world") + '((?u "user" t user "execute a command as another USER") + :raw-positional)))) + (should + (equal '("root" "emerge" "world") + (eshell--process-args + "sudo" + '("-u" "root" "emerge" "-uDN" "world") + '((?u "user" t user "execute a command as another USER") + :raw-positional)))) + (should + (equal '("world" "emerge") + (eshell--process-args + "sudo" + '("-u" "root" "emerge" "-uDN" "world") + '((?u "user" t user "execute a command as another USER")))))) + +(ert-deftest test-eshell-eval-using-options () + "Tests for `eshell-eval-using-options'." + (eshell-eval-using-options + "sudo" '("-u" "root" "whoami") + '((?u "user" t user "execute a command as another USER") + :raw-positional) + (should (equal user "root"))) + (eshell-eval-using-options + "sudo" '("--user" "root" "whoami") + '((?u "user" t user "execute a command as another USER") + :raw-positional) + (should (equal user "root"))) + + (eshell-eval-using-options + "sudo" '("emerge" "-uDN" "world") + '((?u "user" t user "execute a command as another USER")) + (should (equal user "world"))) + (eshell-eval-using-options + "sudo" '("emerge" "-uDN" "world") + '((?u "user" t user "execute a command as another USER") + :raw-positional) + (should (eq user nil))) + + (eshell-eval-using-options + "ls" '("-I" "*.txt" "/dev/null") + '((?I "ignore" t ignore-pattern + "do not list implied entries matching pattern")) + (should (equal ignore-pattern "*.txt"))) + + (eshell-eval-using-options + "ls" '("-l" "/dev/null") + '((?l nil long-listing listing-style + "use a long listing format")) + (should (eql listing-style 'long-listing))) + (eshell-eval-using-options + "ls" '("/dev/null") + '((?l nil long-listing listing-style + "use a long listing format")) + (should (eq listing-style nil))) + + (eshell-eval-using-options + "ls" '("/dev/null" "-h") + '((?h "human-readable" 1024 human-readable + "print sizes in human readable format")) + (should (eql human-readable 1024))) + (eshell-eval-using-options + "ls" '("/dev/null" "--human-readable") + '((?h "human-readable" 1024 human-readable + "print sizes in human readable format")) + (should (eql human-readable 1024))) + (eshell-eval-using-options + "ls" '("/dev/null") + '((?h "human-readable" 1024 human-readable + "print sizes in human readable format")) + (should (eq human-readable nil)))) + +(provide 'esh-opt-tests) + +;;; esh-opt-tests.el ends here -- 2.14.2 --=-=-= Content-Type: text/plain Thanks again for all your amazing work on Emacs! -Jay --=-=-=--