> 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>