From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 29917 invoked by alias); 17 Jul 2014 18:23:50 -0000 Mailing-List: contact zsh-workers-help@zsh.org; run by ezmlm Precedence: bulk X-No-Archive: yes List-Id: Zsh Workers List List-Post: List-Help: X-Seq: 32882 Received: (qmail 14885 invoked from network); 17 Jul 2014 18:23:49 -0000 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on f.primenet.com.au X-Spam-Level: X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.2 From: Bart Schaefer Message-id: <140717112341.ZM3956@torch.brasslantern.com> Date: Thu, 17 Jul 2014 11:23:41 -0700 In-reply-to: <20140717125841.GA27001@arthedain.local> Comments: In reply to Augie Fackler "[PATCH] Fix loading of multi-line history entires from disk" (Jul 17, 8:58am) References: <20140717125841.GA27001@arthedain.local> X-Mailer: OpenZMail Classic (0.9.2 24April2005) To: zsh-workers@zsh.org Subject: Re: [PATCH] Fix loading of multi-line history entires from disk Cc: Augie Fackler MIME-version: 1.0 Content-type: text/plain; charset=us-ascii On Jul 17, 8:58am, Augie Fackler wrote: } Subject: [PATCH] Fix loading of multi-line history entires from disk } } Fixes the issue I posted about yesterday with } history-incremental-search-backward. I have no idea as to the } correctness of the fix, but my history file is written out in a way } that matches 4.3.x with this patch applied, and the behavior matches } again. We've got a hairy problem here. See workers/30432 -- if you write a line that ends in two backslashes, it is NOT rewritten on output, but if you write a line than ends in one backslash then that IS doubled on output ... So either the first case is broken (with Augie's patch and/or prior to workers/30443, it treats that line as continued when it should not) or the second case is broken (it treats continuations as multiple lines). The complication is that a line ending in THREE backslashes (or any odd number of them) needs to go back to being treated as continuation. In that instance there should be a newline embedded in the internal history entry. To sum up: (1) All history files containing any history entries that intentionally end with a double backslash, are broken, for as long as zsh has been writing history files, because those lines are impossible to distinguish from a continuation line where zsh doubled the backslash at output. (2) Zsh versions 5.0.0 through 5.0.5 will incorrectly reload history that contains continuation lines, because of an attempt to fix (1). Versions prior to 5.0.0 will incorrectly load files that have intentional double backslashes, because of (1). (3) Incorrectly reloaded history files will also have been incorrectly written back by versions 5.0.0 through 5.0.5, in any circumstance that saves in extended history format. (4) The only reasonable solution is to fix (1) by changing the way the entries are written, which means old history files will remain broken. Attempting to fix it when loading the files was misguided (my error). So, check out the following. This includes Augie's patch (backs out 30433) and adds an attempt at changing the output (appends a space after an even number of backslashes appearing at the end of an entry). An alternative would be to append a semicolon there. Opinions? Is this approach also misguided, and if so, what do we do? diff --git a/Src/hist.c b/Src/hist.c index 64f88f5..770d559 100644 --- a/Src/hist.c +++ b/Src/hist.c @@ -2308,8 +2308,7 @@ readhistline(int start, char **bufp, int *bufsiz, FILE *in) } else { buf[len - 1] = '\0'; - if (len > 1 && buf[len - 2] == '\\' && - (len < 3 || buf[len - 3] != '\\')) { + if (len > 1 && buf[len - 2] == '\\') { buf[--len - 1] = '\n'; if (!feof(in)) return readhistline(len, bufp, bufsiz, in); @@ -2618,6 +2617,8 @@ savehistfile(char *fn, int err, int writeflags) ret = 0; for (; he && he->histnum <= xcurhist; he = down_histent(he)) { + int count_backslashes = 0; + if ((writeflags & HFILE_SKIPDUPS && he->node.flags & HIST_DUP) || (writeflags & HFILE_SKIPFOREIGN && he->node.flags & HIST_FOREIGN) || he->node.flags & HIST_TMPSTORE) @@ -2649,9 +2650,18 @@ savehistfile(char *fn, int err, int writeflags) if (*t == '\n') if ((ret = fputc('\\', out)) < 0) break; + if (*t == '\\') + count_backslashes++; + else + count_backslashes = 0; if ((ret = fputc(*t, out)) < 0) break; } + if (ret < 0) + break; + if (count_backslashes && (count_backslashes % 2 == 0)) + if ((ret = fputc(' ', out)) < 0) + break; if (ret < 0 || (ret = fputc('\n', out)) < 0) break; }