unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#36034: [PATCH] Zsh extended_history shows up in comint input ring
@ 2019-05-31 20:30 Matthew Bauer
  2019-06-23 16:53 ` Lars Ingebrigtsen
  0 siblings, 1 reply; 12+ messages in thread
From: Matthew Bauer @ 2019-05-31 20:30 UTC (permalink / raw)
  To: 36034

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

Currently, Zsh’s extended_history option is not handled well in Emacs.
The comint buffer does not know to skip it when running
comint-read-input-ring. The attached patch handles this.

This behavior is described in the Zsh manual available at:

http://zsh.sourceforge.net/Doc/Release/Options.html#History

The format of this line looks like this:

: <beginning time>:<elapsed seconds>;<command>

This patch just skips those timestamp to get the <command> part.


[-- Attachment #2: 0001-Add-zsh-extended_history-handling-for-comint.el-inpu.patch --]
[-- Type: text/x-patch, Size: 1607 bytes --]

From b8a8857cd686fae1ebbeca79f4469ce878837b90 Mon Sep 17 00:00:00 2001
From: Matthew Bauer <mjbauer95@gmail.com>
Date: Fri, 31 May 2019 16:27:24 -0400
Subject: [PATCH] Add zsh extended_history handling for comint.el input ring
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Adds handling of the Zsh extended_history to comint.el input
ring. This means that the timestamp doesn’t show up when reading
through history from other shells. The lines look like this:

: <beginning time>:<elapsed seconds>;<command>

This patch skips the part before <command>.

Zsh documents it here:

http://zsh.sourceforge.net/Doc/Release/Options.html#History
---
 lisp/comint.el | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/lisp/comint.el b/lisp/comint.el
index 3dce1c9c8d..c5c0ad0f7b 100644
--- a/lisp/comint.el
+++ b/lisp/comint.el
@@ -976,7 +976,11 @@ See also `comint-input-ignoredups' and `comint-write-input-ring'."
                  (setq start
                        (if (re-search-backward comint-input-ring-separator
                                                nil t)
-                           (match-end 0)
+                           (progn
+                             ;; Skip zsh extended_history stamps
+                             (re-search-forward ": [[:digit:]]+:[[:digit:]]+;" nil t)
+
+                             (match-end 0))
                          (point-min)))
                  (setq history (buffer-substring start end))
                  (goto-char start)
-- 
2.21.0


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

* bug#36034: [PATCH] Zsh extended_history shows up in comint input ring
  2019-05-31 20:30 bug#36034: [PATCH] Zsh extended_history shows up in comint input ring Matthew Bauer
@ 2019-06-23 16:53 ` Lars Ingebrigtsen
  2019-06-23 22:27   ` Matthew Bauer
  0 siblings, 1 reply; 12+ messages in thread
From: Lars Ingebrigtsen @ 2019-06-23 16:53 UTC (permalink / raw)
  To: Matthew Bauer; +Cc: 36034

Matthew Bauer <mjbauer95@gmail.com> writes:

> The format of this line looks like this:
>
> : <beginning time>:<elapsed seconds>;<command>
>
> This patch just skips those timestamp to get the <command> part.

[...]

> --- a/lisp/comint.el
> +++ b/lisp/comint.el
> @@ -976,7 +976,11 @@ See also `comint-input-ignoredups' and `comint-write-input-ring'."
>                   (setq start
>                         (if (re-search-backward comint-input-ring-separator
>                                                 nil t)
> -                           (match-end 0)
> +                           (progn
> +                             ;; Skip zsh extended_history stamps
> +                             (re-search-forward ": [[:digit:]]+:[[:digit:]]+;" nil t)
> +
> +                             (match-end 0))
>                           (point-min)))
>                   (setq history (buffer-substring start end))
>                   (goto-char start)

I'm not that familiar with the comint/shell code...  but this is done
in the central comint code, so it would do this for all the modes that
use comint?  Couldn't that lead to problems in these other modes that
aren't doing this timestamp thing?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





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

* bug#36034: [PATCH] Zsh extended_history shows up in comint input ring
  2019-06-23 16:53 ` Lars Ingebrigtsen
@ 2019-06-23 22:27   ` Matthew Bauer
  2019-06-24 10:51     ` Lars Ingebrigtsen
  2019-06-24 10:51     ` Lars Ingebrigtsen
  0 siblings, 2 replies; 12+ messages in thread
From: Matthew Bauer @ 2019-06-23 22:27 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 36034

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


> On Jun 23, 2019, at 12:53 PM, Lars Ingebrigtsen <larsi@gnus.org> wrote:
> 
> I'm not that familiar with the comint/shell code...  but this is done
> in the central comint code, so it would do this for all the modes that
> use comint?  Couldn't that lead to problems in these other modes that
> aren't doing this timestamp thing?

Yes, it does apply for any comint buffer that uses comint-read-input-ring. i had originally thought this was only used by shell-mode, but you are correct that it is used by others. I’ve added defcustom handling to the patch to properly account for this. In addition, I have made some revisions to better account for the extended history prefix. Any additional help on this would be appreciate!



[-- Attachment #2: 0001-Add-zsh-extended_history-handling-for-comint.el-inpu.patch --]
[-- Type: application/octet-stream, Size: 4027 bytes --]

From 54a0aa6a4de95f898cf13c67a5eb698dbfd9a46b Mon Sep 17 00:00:00 2001
From: Matthew Bauer <mjbauer95@gmail.com>
Date: Sun, 23 Jun 2019 18:08:32 -0400
Subject: [PATCH] Add zsh extended_history handling for comint.el input ring
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Adds handling of the Zsh extended_history to comint.el input
ring. This means that the timestamp doesn’t show up when reading
through history from other shells. The lines look like this:

: <beginning time>:<elapsed seconds>;<command>

This patch skips the part before <command> when the ‘shell’ is
zsh. This is done through the comint-input-ring-file-prefix
variable. When set, this will skip a prefix of the file when loading
into the input ring. This matches the behavior of Zsh. Zsh documents
it here:

http://zsh.sourceforge.net/Doc/Release/Options.html#History
---
 lisp/comint.el | 26 ++++++++++++++++++++++++--
 lisp/shell.el  |  6 +++++-
 2 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/lisp/comint.el b/lisp/comint.el
index 56e38e24ac..fef874387a 100644
--- a/lisp/comint.el
+++ b/lisp/comint.el
@@ -249,6 +249,12 @@ to set this in a mode hook, rather than customize the default value."
 		 file)
   :group 'comint)
 
+(defcustom comint-input-ring-file-prefix nil
+  "If non-nil, the prefix to skip when parsing the input ring file.
+This is useful in Zsh when the extended_history option is on."
+  :type 'boolean
+  :group 'comint)
+
 (defcustom comint-scroll-to-bottom-on-input nil
   "Controls whether input to interpreter causes window to scroll.
 If nil, then do not scroll.  If t or `all', scroll all windows showing buffer.
@@ -978,8 +984,24 @@ See also `comint-input-ignoredups' and `comint-write-input-ring'."
                  (setq start
                        (if (re-search-backward comint-input-ring-separator
                                                nil t)
-                           (match-end 0)
-                         (point-min)))
+                           (progn
+                             (when (and
+                                    comint-input-ring-file-prefix
+                                    (looking-at (concat comint-input-ring-separator
+                                                        comint-input-ring-file-prefix)))
+                               ;; Skip zsh extended_history stamps
+                               (re-search-forward comint-input-ring-file-prefix
+                                                  nil t))
+                             (match-end 0))
+                         (progn
+                           (goto-char (point-min))
+                           (if (and comint-input-ring-file-prefix
+                                    (looking-at comint-input-ring-file-prefix))
+                               (progn
+                                 (re-search-forward comint-input-ring-file-prefix
+                                                    nil t)
+                                 (match-end 0))
+                             (point-min)))))
                  (setq history (buffer-substring start end))
                  (goto-char start)
                  (when (and (not (string-match comint-input-history-ignore
diff --git a/lisp/shell.el b/lisp/shell.el
index e30825cd66..25f6892d96 100644
--- a/lisp/shell.el
+++ b/lisp/shell.el
@@ -600,7 +600,11 @@ buffer."
       ;; Bypass a bug in certain versions of bash.
       (when (string-equal shell "bash")
         (add-hook 'comint-preoutput-filter-functions
-                  'shell-filter-ctrl-a-ctrl-b nil t)))
+                  'shell-filter-ctrl-a-ctrl-b nil t))
+
+      ;; Skip extended history for zsh.
+      (when (string-equal shell "zsh")
+        (setq-local comint-input-ring-file-prefix ": [[:digit:]]+:[[:digit:]]+;")))
     (comint-read-input-ring t)))
 
 (defun shell-apply-ansi-color (beg end face)
-- 
2.21.0


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

* bug#36034: [PATCH] Zsh extended_history shows up in comint input ring
  2019-06-23 22:27   ` Matthew Bauer
  2019-06-24 10:51     ` Lars Ingebrigtsen
@ 2019-06-24 10:51     ` Lars Ingebrigtsen
  1 sibling, 0 replies; 12+ messages in thread
From: Lars Ingebrigtsen @ 2019-06-24 10:51 UTC (permalink / raw)
  To: Matthew Bauer; +Cc: 36034

Matthew Bauer <mjbauer95@gmail.com> writes:

> Yes, it does apply for any comint buffer that uses
> comint-read-input-ring. i had originally thought this was only used by
> shell-mode, but you are correct that it is used by others. Ive added
> defcustom handling to the patch to properly account for this.

Looks less dangerous now.  :-)

Some comments:

> +(defcustom comint-input-ring-file-prefix nil
> +  "If non-nil, the prefix to skip when parsing the input ring file.
> +This is useful in Zsh when the extended_history option is on."
> +  :type 'boolean
> +  :group 'comint)

This doesn't really seem like a user-level variable, so it should just
be a defvar, I think.

>                   (setq start
>                         (if (re-search-backward comint-input-ring-separator
>                                                 nil t)
> -                           (match-end 0)
> -                         (point-min)))
> +                           (progn
> +                             (when (and
> +                                    comint-input-ring-file-prefix
> +                                    (looking-at (concat comint-input-ring-separator
> +                                                        comint-input-ring-file-prefix)))
> +                               ;; Skip zsh extended_history stamps
> +                               (re-search-forward comint-input-ring-file-prefix
> +                                                  nil t))
> +                             (match-end 0))

The re-search-forward here doesn't seem necessary -- can't you just go
to (match-end 0) here instead?

> +                         (progn
> +                           (goto-char (point-min))
> +                           (if (and comint-input-ring-file-prefix
> +                                    (looking-at comint-input-ring-file-prefix))
> +                               (progn
> +                                 (re-search-forward comint-input-ring-file-prefix
> +                                                    nil t)
> +                                 (match-end 0))
> +                             (point-min)))))

And I don't understand this bit.  This is when we didn't find
comint-input-ring-separator, right?  But you still want to skip
comint-input-ring-file-prefix?

If you want to skip it anyway, then you can just have the check (and the
skip) after the if statement...

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





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

* bug#36034: [PATCH] Zsh extended_history shows up in comint input ring
  2019-06-23 22:27   ` Matthew Bauer
@ 2019-06-24 10:51     ` Lars Ingebrigtsen
  2019-06-24 22:45       ` Matthew Bauer
  2019-06-24 10:51     ` Lars Ingebrigtsen
  1 sibling, 1 reply; 12+ messages in thread
From: Lars Ingebrigtsen @ 2019-06-24 10:51 UTC (permalink / raw)
  To: Matthew Bauer; +Cc: 36034

Matthew Bauer <mjbauer95@gmail.com> writes:

> Yes, it does apply for any comint buffer that uses
> comint-read-input-ring. i had originally thought this was only used by
> shell-mode, but you are correct that it is used by others. Ive added
> defcustom handling to the patch to properly account for this.

Looks less dangerous now.  :-)

Some comments:

> +(defcustom comint-input-ring-file-prefix nil
> +  "If non-nil, the prefix to skip when parsing the input ring file.
> +This is useful in Zsh when the extended_history option is on."
> +  :type 'boolean
> +  :group 'comint)

This doesn't really seem like a user-level variable, so it should just
be a defvar, I think.

>                   (setq start
>                         (if (re-search-backward comint-input-ring-separator
>                                                 nil t)
> -                           (match-end 0)
> -                         (point-min)))
> +                           (progn
> +                             (when (and
> +                                    comint-input-ring-file-prefix
> +                                    (looking-at (concat comint-input-ring-separator
> +                                                        comint-input-ring-file-prefix)))
> +                               ;; Skip zsh extended_history stamps
> +                               (re-search-forward comint-input-ring-file-prefix
> +                                                  nil t))
> +                             (match-end 0))

The re-search-forward here doesn't seem necessary -- can't you just go
to (match-end 0) here instead?

> +                         (progn
> +                           (goto-char (point-min))
> +                           (if (and comint-input-ring-file-prefix
> +                                    (looking-at comint-input-ring-file-prefix))
> +                               (progn
> +                                 (re-search-forward comint-input-ring-file-prefix
> +                                                    nil t)
> +                                 (match-end 0))
> +                             (point-min)))))

And I don't understand this bit.  This is when we didn't find
comint-input-ring-separator, right?  But you still want to skip
comint-input-ring-file-prefix?

If you want to skip it anyway, then you can just have the check (and the
skip) after the if statement...

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





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

* bug#36034: [PATCH] Zsh extended_history shows up in comint input ring
  2019-06-24 10:51     ` Lars Ingebrigtsen
@ 2019-06-24 22:45       ` Matthew Bauer
  2019-06-25 11:05         ` Lars Ingebrigtsen
  0 siblings, 1 reply; 12+ messages in thread
From: Matthew Bauer @ 2019-06-24 22:45 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 36034

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



> On Jun 24, 2019, at 6:51 AM, Lars Ingebrigtsen <larsi@gnus.org> wrote:
> 
> Matthew Bauer <mjbauer95@gmail.com> writes:
> 
>> +(defcustom comint-input-ring-file-prefix nil
>> +  "If non-nil, the prefix to skip when parsing the input ring file.
>> +This is useful in Zsh when the extended_history option is on."
>> +  :type 'boolean
>> +  :group 'comint)
> 
> This doesn't really seem like a user-level variable, so it should just
> be a defvar, I think.

Seems reasonable, I updated the patch.

> 
>>                  (setq start
>>                        (if (re-search-backward comint-input-ring-separator
>>                                                nil t)
>> -                           (match-end 0)
>> -                         (point-min)))
>> +                           (progn
>> +                             (when (and
>> +                                    comint-input-ring-file-prefix
>> +                                    (looking-at (concat comint-input-ring-separator
>> +                                                        comint-input-ring-file-prefix)))
>> +                               ;; Skip zsh extended_history stamps
>> +                               (re-search-forward comint-input-ring-file-prefix
>> +                                                  nil t))
>> +                             (match-end 0))
> 
> The re-search-forward here doesn't seem necessary -- can't you just go
> to (match-end 0) here instead?

Without re-search-forward, the “start” integer would just be the character right after the newline. re-search-forward skips that prefix.

>> +                         (progn
>> +                           (goto-char (point-min))
>> +                           (if (and comint-input-ring-file-prefix
>> +                                    (looking-at comint-input-ring-file-prefix))
>> +                               (progn
>> +                                 (re-search-forward comint-input-ring-file-prefix
>> +                                                    nil t)
>> +                                 (match-end 0))
>> +                             (point-min)))))
> 
> And I don't understand this bit.  This is when we didn't find
> comint-input-ring-separator, right?  But you still want to skip
> comint-input-ring-file-prefix?
> 
> If you want to skip it anyway, then you can just have the check (and the
> skip) after the if statement…
> 

This is for the very first line which will not have a newline before it. In this case, we want to skip the prefix as well. This is part of why we need special handling for “prefix" and tricks like this don’t work:

https://emacs.stackexchange.com/a/22295 <https://emacs.stackexchange.com/a/22295>


[-- Attachment #2.1: Type: text/html, Size: 7920 bytes --]

[-- Attachment #2.2: 0001-Add-zsh-extended_history-handling-for-comint.el-inpu.patch --]
[-- Type: application/octet-stream, Size: 3972 bytes --]

From ad957ff66cb9d48a1ae7772758b40e534392f097 Mon Sep 17 00:00:00 2001
From: Matthew Bauer <mjbauer95@gmail.com>
Date: Sun, 23 Jun 2019 18:08:32 -0400
Subject: [PATCH] Add zsh extended_history handling for comint.el input ring
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Adds handling of the Zsh extended_history to comint.el input
ring. This means that the timestamp doesn’t show up when reading
through history from other shells. The lines look like this:

: <beginning time>:<elapsed seconds>;<command>

This patch skips the part before <command> when the ‘shell’ is
zsh. This is done through the comint-input-ring-file-prefix
variable. When set, this will skip a prefix of the file when loading
into the input ring. This matches the behavior of Zsh. Zsh documents
it here:

http://zsh.sourceforge.net/Doc/Release/Options.html#History
---
 lisp/comint.el | 24 ++++++++++++++++++++++--
 lisp/shell.el  |  6 +++++-
 2 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/lisp/comint.el b/lisp/comint.el
index 56e38e24ac..dbf239b463 100644
--- a/lisp/comint.el
+++ b/lisp/comint.el
@@ -249,6 +249,10 @@ to set this in a mode hook, rather than customize the default value."
 		 file)
   :group 'comint)
 
+(defvar comint-input-ring-file-prefix nil
+  "The prefix to skip when parsing the input ring file.
+This is useful in Zsh when the extended_history option is on.")
+
 (defcustom comint-scroll-to-bottom-on-input nil
   "Controls whether input to interpreter causes window to scroll.
 If nil, then do not scroll.  If t or `all', scroll all windows showing buffer.
@@ -978,8 +982,24 @@ See also `comint-input-ignoredups' and `comint-write-input-ring'."
                  (setq start
                        (if (re-search-backward comint-input-ring-separator
                                                nil t)
-                           (match-end 0)
-                         (point-min)))
+                           (progn
+                             (when (and
+                                    comint-input-ring-file-prefix
+                                    (looking-at (concat comint-input-ring-separator
+                                                        comint-input-ring-file-prefix)))
+                               ;; Skip zsh extended_history stamps
+                               (re-search-forward comint-input-ring-file-prefix
+                                                  nil t))
+                             (match-end 0))
+                         (progn
+                           (goto-char (point-min))
+                           (if (and comint-input-ring-file-prefix
+                                    (looking-at comint-input-ring-file-prefix))
+                               (progn
+                                 (re-search-forward comint-input-ring-file-prefix
+                                                    nil t)
+                                 (match-end 0))
+                             (point-min)))))
                  (setq history (buffer-substring start end))
                  (goto-char start)
                  (when (and (not (string-match comint-input-history-ignore
diff --git a/lisp/shell.el b/lisp/shell.el
index e30825cd66..25f6892d96 100644
--- a/lisp/shell.el
+++ b/lisp/shell.el
@@ -600,7 +600,11 @@ buffer."
       ;; Bypass a bug in certain versions of bash.
       (when (string-equal shell "bash")
         (add-hook 'comint-preoutput-filter-functions
-                  'shell-filter-ctrl-a-ctrl-b nil t)))
+                  'shell-filter-ctrl-a-ctrl-b nil t))
+
+      ;; Skip extended history for zsh.
+      (when (string-equal shell "zsh")
+        (setq-local comint-input-ring-file-prefix ": [[:digit:]]+:[[:digit:]]+;")))
     (comint-read-input-ring t)))
 
 (defun shell-apply-ansi-color (beg end face)
-- 
2.21.0


[-- Attachment #2.3: Type: text/html, Size: 206 bytes --]

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

* bug#36034: [PATCH] Zsh extended_history shows up in comint input ring
  2019-06-24 22:45       ` Matthew Bauer
@ 2019-06-25 11:05         ` Lars Ingebrigtsen
  2019-06-29 21:35           ` Matthew Bauer
  0 siblings, 1 reply; 12+ messages in thread
From: Lars Ingebrigtsen @ 2019-06-25 11:05 UTC (permalink / raw)
  To: Matthew Bauer; +Cc: 36034

Matthew Bauer <mjbauer95@gmail.com> writes:

>  -                           (match-end 0)
>  -                         (point-min)))
>  +                           (progn
>  +                             (when (and
>  +                                    comint-input-ring-file-prefix
>  +                                    (looking-at (concat
>  comint-input-ring-separator
>  +                                                        comint-input-ring-file-prefix)))
>  +                               ;; Skip zsh extended_history stamps
>  +                               (re-search-forward comint-input-ring-file-prefix
>  +                                                  nil t))
>  +                             (match-end 0))
>
>  The re-search-forward here doesn't seem necessary -- can't you just go
>  to (match-end 0) here instead?
>
> Without re-search-forward, the “start” integer would just be the character
> right after the newline. re-search-forward skips that prefix.

You first do a

(looking-at (concat comint-input-ring-separator comint-input-ring-file-prefix)))

and then

(re-search-forward comint-input-ring-file-prefix)

This will make point end up at where (match-end 0) would be after the
looking-at, surely?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





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

* bug#36034: [PATCH] Zsh extended_history shows up in comint input ring
  2019-06-25 11:05         ` Lars Ingebrigtsen
@ 2019-06-29 21:35           ` Matthew Bauer
  2019-07-04 13:37             ` Lars Ingebrigtsen
  0 siblings, 1 reply; 12+ messages in thread
From: Matthew Bauer @ 2019-06-29 21:35 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 36034

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


> On Jun 25, 2019, at 7:05 AM, Lars Ingebrigtsen <larsi@gnus.org> wrote:

> This will make point end up at where (match-end 0) would be after the
> looking-at, surely?
> 

Ok yeah that simplified things a bit. Attached an updated version.


[-- Attachment #2: 0001-Add-zsh-extended_history-handling-for-comint.el-inpu.patch --]
[-- Type: application/octet-stream, Size: 3557 bytes --]

From 286ceba115b3deabfd90e589c3dbebf03a992e25 Mon Sep 17 00:00:00 2001
From: Matthew Bauer <mjbauer95@gmail.com>
Date: Sun, 23 Jun 2019 18:08:32 -0400
Subject: [PATCH] Add zsh extended_history handling for comint.el input ring
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Adds handling of the Zsh extended_history to comint.el input
ring. This means that the timestamp doesn’t show up when reading
through history from other shells. The lines look like this:

: <beginning time>:<elapsed seconds>;<command>

This patch skips the part before <command> when the ‘shell’ is
zsh. This is done through the comint-input-ring-file-prefix
variable. When set, this will skip a prefix of the file when loading
into the input ring. This matches the behavior of Zsh. Zsh documents
it here:

http://zsh.sourceforge.net/Doc/Release/Options.html#History
---
 lisp/comint.el | 18 ++++++++++++++++--
 lisp/shell.el  |  6 +++++-
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/lisp/comint.el b/lisp/comint.el
index 56e38e24ac..6d4293b722 100644
--- a/lisp/comint.el
+++ b/lisp/comint.el
@@ -249,6 +249,10 @@ to set this in a mode hook, rather than customize the default value."
 		 file)
   :group 'comint)
 
+(defvar comint-input-ring-file-prefix nil
+  "The prefix to skip when parsing the input ring file.
+This is useful in Zsh when the extended_history option is on.")
+
 (defcustom comint-scroll-to-bottom-on-input nil
   "Controls whether input to interpreter causes window to scroll.
 If nil, then do not scroll.  If t or `all', scroll all windows showing buffer.
@@ -978,8 +982,18 @@ See also `comint-input-ignoredups' and `comint-write-input-ring'."
                  (setq start
                        (if (re-search-backward comint-input-ring-separator
                                                nil t)
-                           (match-end 0)
-                         (point-min)))
+                           (progn
+                             (when comint-input-ring-file-prefix
+                               ;; Skip zsh extended_history stamps
+                               (re-search-forward comint-input-ring-file-prefix
+                                                  nil t))
+                             (match-end 0))
+                         (progn
+                           (goto-char (point-min))
+                           (when comint-input-ring-file-prefix
+                             (re-search-forward comint-input-ring-file-prefix
+                                                nil t))
+                           (point))))
                  (setq history (buffer-substring start end))
                  (goto-char start)
                  (when (and (not (string-match comint-input-history-ignore
diff --git a/lisp/shell.el b/lisp/shell.el
index e30825cd66..25f6892d96 100644
--- a/lisp/shell.el
+++ b/lisp/shell.el
@@ -600,7 +600,11 @@ buffer."
       ;; Bypass a bug in certain versions of bash.
       (when (string-equal shell "bash")
         (add-hook 'comint-preoutput-filter-functions
-                  'shell-filter-ctrl-a-ctrl-b nil t)))
+                  'shell-filter-ctrl-a-ctrl-b nil t))
+
+      ;; Skip extended history for zsh.
+      (when (string-equal shell "zsh")
+        (setq-local comint-input-ring-file-prefix ": [[:digit:]]+:[[:digit:]]+;")))
     (comint-read-input-ring t)))
 
 (defun shell-apply-ansi-color (beg end face)
-- 
2.22.0


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

* bug#36034: [PATCH] Zsh extended_history shows up in comint input ring
  2019-06-29 21:35           ` Matthew Bauer
@ 2019-07-04 13:37             ` Lars Ingebrigtsen
  2020-01-20 19:43               ` Stefan Kangas
  2020-03-18 15:05               ` Matthew Bauer
  0 siblings, 2 replies; 12+ messages in thread
From: Lars Ingebrigtsen @ 2019-07-04 13:37 UTC (permalink / raw)
  To: Matthew Bauer; +Cc: 36034

Matthew Bauer <mjbauer95@gmail.com> writes:

>                   (setq start
>                         (if (re-search-backward comint-input-ring-separator
>                                                 nil t)
> -                           (match-end 0)
> -                         (point-min)))
> +                           (progn
> +                             (when comint-input-ring-file-prefix
> +                               ;; Skip zsh extended_history stamps
> +                               (re-search-forward comint-input-ring-file-prefix
> +                                                  nil t))
> +                             (match-end 0))

Hm...  I don't think this is right, either.  If the re-search-forward
fails, then (match-end 0) will fail, too.  And since (if I understood
correctly), the prefix will follow on directly from where point it,
using looking-at would be better, anyway...

>        ;; Bypass a bug in certain versions of bash.
>        (when (string-equal shell "bash")
>          (add-hook 'comint-preoutput-filter-functions
> -                  'shell-filter-ctrl-a-ctrl-b nil t)))
> +                  'shell-filter-ctrl-a-ctrl-b nil t))
> +
> +      ;; Skip extended history for zsh.
> +      (when (string-equal shell "zsh")
> +        (setq-local comint-input-ring-file-prefix ": [[:digit:]]+:[[:digit:]]+;")))
>      (comint-read-input-ring t)))

And this bit didn't apply.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





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

* bug#36034: [PATCH] Zsh extended_history shows up in comint input ring
  2019-07-04 13:37             ` Lars Ingebrigtsen
@ 2020-01-20 19:43               ` Stefan Kangas
  2020-03-18 15:05               ` Matthew Bauer
  1 sibling, 0 replies; 12+ messages in thread
From: Stefan Kangas @ 2020-01-20 19:43 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: Matthew Bauer, 36034

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Matthew Bauer <mjbauer95@gmail.com> writes:
>
>>                   (setq start
>>                         (if (re-search-backward comint-input-ring-separator
>>                                                 nil t)
>> -                           (match-end 0)
>> -                         (point-min)))
>> +                           (progn
>> +                             (when comint-input-ring-file-prefix
>> +                               ;; Skip zsh extended_history stamps
>> +                               (re-search-forward comint-input-ring-file-prefix
>> +                                                  nil t))
>> +                             (match-end 0))
>
> Hm...  I don't think this is right, either.  If the re-search-forward
> fails, then (match-end 0) will fail, too.  And since (if I understood
> correctly), the prefix will follow on directly from where point it,
> using looking-at would be better, anyway...
>
>>        ;; Bypass a bug in certain versions of bash.
>>        (when (string-equal shell "bash")
>>          (add-hook 'comint-preoutput-filter-functions
>> -                  'shell-filter-ctrl-a-ctrl-b nil t)))
>> +                  'shell-filter-ctrl-a-ctrl-b nil t))
>> +
>> +      ;; Skip extended history for zsh.
>> +      (when (string-equal shell "zsh")
>> +        (setq-local comint-input-ring-file-prefix ": [[:digit:]]+:[[:digit:]]+;")))
>>      (comint-read-input-ring t)))
>
> And this bit didn't apply.

That was 7 months ago.  Did you have any time to look into the
comments by Lars above?  Thanks.

Best regards,
Stefan Kangas





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

* bug#36034: [PATCH] Zsh extended_history shows up in comint input ring
  2019-07-04 13:37             ` Lars Ingebrigtsen
  2020-01-20 19:43               ` Stefan Kangas
@ 2020-03-18 15:05               ` Matthew Bauer
  2020-08-10 11:11                 ` Lars Ingebrigtsen
  1 sibling, 1 reply; 12+ messages in thread
From: Matthew Bauer @ 2020-03-18 15:05 UTC (permalink / raw)
  To: Lars Ingebrigtsen, Stefan Kangas, 36034


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

Sorry for the late reply. Attached is the updated patch, using looking-at
and on top of latest Emacs master.

On Thu, Jul 4, 2019 at 9:37 AM Lars Ingebrigtsen <larsi@gnus.org> wrote:

> Matthew Bauer <mjbauer95@gmail.com> writes:
>
> >                   (setq start
> >                         (if (re-search-backward
> comint-input-ring-separator
> >                                                 nil t)
> > -                           (match-end 0)
> > -                         (point-min)))
> > +                           (progn
> > +                             (when comint-input-ring-file-prefix
> > +                               ;; Skip zsh extended_history stamps
> > +                               (re-search-forward
> comint-input-ring-file-prefix
> > +                                                  nil t))
> > +                             (match-end 0))
>
> Hm...  I don't think this is right, either.  If the re-search-forward
> fails, then (match-end 0) will fail, too.  And since (if I understood
> correctly), the prefix will follow on directly from where point it,
> using looking-at would be better, anyway...
>
> >        ;; Bypass a bug in certain versions of bash.
> >        (when (string-equal shell "bash")
> >          (add-hook 'comint-preoutput-filter-functions
> > -                  'shell-filter-ctrl-a-ctrl-b nil t)))
> > +                  'shell-filter-ctrl-a-ctrl-b nil t))
> > +
> > +      ;; Skip extended history for zsh.
> > +      (when (string-equal shell "zsh")
> > +        (setq-local comint-input-ring-file-prefix ":
> [[:digit:]]+:[[:digit:]]+;")))
> >      (comint-read-input-ring t)))
>
> And this bit didn't apply.
>
> --
> (domestic pets only, the antidote for overdose, milk.)
>    bloggy blog: http://lars.ingebrigtsen.no
>

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

[-- Attachment #2: 0001-Add-zsh-extended_history-handling-for-comint.el-inpu.patch --]
[-- Type: text/x-patch, Size: 3653 bytes --]

From 042754dfa875a1bd169b6ff2247ec94c75e1f789 Mon Sep 17 00:00:00 2001
From: Matthew Bauer <mjbauer95@gmail.com>
Date: Wed, 18 Mar 2020 10:59:45 -0400
Subject: [PATCH] Add zsh extended_history handling for comint.el input ring
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Adds handling of the Zsh extended_history to comint.el input
ring. This means that the timestamp doesn’t show up when reading
through history from other shells. The lines look like this:

: <beginning time>:<elapsed seconds>;<command>

This patch skips the part before <command> when the ‘shell’ is
zsh. This is done through the comint-input-ring-file-prefix
variable. When set, this will skip a prefix of the file when loading
into the input ring. This matches the behavior of Zsh. Zsh documents
it here:

http://zsh.sourceforge.net/Doc/Release/Options.html#History
---
 lisp/comint.el | 20 ++++++++++++++++++--
 lisp/shell.el  |  6 +++++-
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/lisp/comint.el b/lisp/comint.el
index ea06f8af87..03898c3027 100644
--- a/lisp/comint.el
+++ b/lisp/comint.el
@@ -249,6 +249,10 @@ to set this in a mode hook, rather than customize the default value."
 		 file)
   :group 'comint)
 
+(defvar comint-input-ring-file-prefix nil
+  "The prefix to skip when parsing the input ring file.
+This is useful in Zsh when the extended_history option is on.")
+
 (defcustom comint-scroll-to-bottom-on-input nil
   "Controls whether input to interpreter causes window to scroll.
 If nil, then do not scroll.  If t or `all', scroll all windows showing buffer.
@@ -987,8 +991,20 @@ See also `comint-input-ignoredups' and `comint-write-input-ring'."
                            (setq end (match-beginning 0)))
                  (setq start
                        (if (re-search-backward ring-separator nil t)
-                           (match-end 0)
-                         (point-min)))
+                           (progn
+                             (when (and comint-input-ring-file-prefix
+                                        (looking-at comint-input-ring-file-prefix))
+                               ;; Skip zsh extended_history stamps
+                               (re-search-forward comint-input-ring-file-prefix
+                                                  nil t))
+                             (match-end 0))
+                         (progn
+                           (goto-char (point-min))
+                           (when (and comint-input-ring-file-prefix
+                                      (looking-at comint-input-ring-file-prefix))
+                             (re-search-forward comint-input-ring-file-prefix
+                                                nil t))
+                           (point))))
                  (setq history (buffer-substring start end))
                  (goto-char start)
                  (when (and (not (string-match history-ignore history))
diff --git a/lisp/shell.el b/lisp/shell.el
index 1e2679f723..3a63949821 100644
--- a/lisp/shell.el
+++ b/lisp/shell.el
@@ -622,7 +622,11 @@ buffer."
       ;; Bypass a bug in certain versions of bash.
       (when (string-equal shell "bash")
         (add-hook 'comint-preoutput-filter-functions
-                  #'shell-filter-ctrl-a-ctrl-b nil t)))
+                  #'shell-filter-ctrl-a-ctrl-b nil t))
+
+      ;; Skip extended history for zsh.
+      (when (string-equal shell "zsh")
+        (setq-local comint-input-ring-file-prefix ": [[:digit:]]+:[[:digit:]]+;")))
     (comint-read-input-ring t)))
 
 (defun shell-apply-ansi-color (beg end face)
-- 
2.23.1


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

* bug#36034: [PATCH] Zsh extended_history shows up in comint input ring
  2020-03-18 15:05               ` Matthew Bauer
@ 2020-08-10 11:11                 ` Lars Ingebrigtsen
  0 siblings, 0 replies; 12+ messages in thread
From: Lars Ingebrigtsen @ 2020-08-10 11:11 UTC (permalink / raw)
  To: Matthew Bauer; +Cc: Stefan Kangas, 36034

Matthew Bauer <mjbauer95@gmail.com> writes:

> Adds handling of the Zsh extended_history to comint.el input
> ring. This means that the timestamp doesn’t show up when reading
> through history from other shells. The lines look like this:
>
> : <beginning time>:<elapsed seconds>;<command>

Thanks; I applied your patch (with some changes -- I replaced a couple
of re-search-forwards that didn't seem to do much more than (goto
(match-end 0))).

Please check that I didn't mess something up when doing that fix.  :-)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





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

end of thread, other threads:[~2020-08-10 11:11 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-05-31 20:30 bug#36034: [PATCH] Zsh extended_history shows up in comint input ring Matthew Bauer
2019-06-23 16:53 ` Lars Ingebrigtsen
2019-06-23 22:27   ` Matthew Bauer
2019-06-24 10:51     ` Lars Ingebrigtsen
2019-06-24 22:45       ` Matthew Bauer
2019-06-25 11:05         ` Lars Ingebrigtsen
2019-06-29 21:35           ` Matthew Bauer
2019-07-04 13:37             ` Lars Ingebrigtsen
2020-01-20 19:43               ` Stefan Kangas
2020-03-18 15:05               ` Matthew Bauer
2020-08-10 11:11                 ` Lars Ingebrigtsen
2019-06-24 10:51     ` Lars Ingebrigtsen

Code repositories for project(s) associated with this public inbox

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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).