zsh-workers
 help / color / mirror / code / Atom feed
From: Mikael Magnusson <mikachu@gmail.com>
To: Peter Stephenson <p.stephenson@samsung.com>
Cc: "Zsh Hackers' List" <zsh-workers@zsh.org>
Subject: Re: PATCH: parse from even deeper in hell
Date: Mon, 23 Feb 2015 12:35:51 +0100	[thread overview]
Message-ID: <CAHYJk3R-K=WX=L4qPD207wZaxoE1K-gGxbaywujhTjzXv0fe9w@mail.gmail.com> (raw)
In-Reply-To: <20150223101157.08acf00c@pwslap01u.europe.root.pri>

> Apparently this message from Mikael didn't go to the list...

I made a draft last night before going to bed, and decided I was too
tired to send, apparently when I woke up I was still too tired to
check if I had set reply-to-all. :)

On Mon, Feb 23, 2015 at 11:11 AM, Peter Stephenson
<p.stephenson@samsung.com> wrote:
>> > Mikael Magnusson <mikachu@gmail.com> wrote:
>> I actually thought the patch in 34587 had fixed it, turns out I just
>> lost my --enable-zsh-debug flag. This patch doesn't fix it either
>> though, and I get a different set of errors every time I open a
>> terminal now, so that's fun. Which is to say it's all the same
>> wordsplit error, but it's printed for a different set of lines. Some
>> of them are the same though.
>
> Obviously, if you want to go down that route, you'll have to look at see
> what's up with those lines.  It seemed to work on the line you originall
> gave.
>
>> I noticed another thing from these errors too, they're printed also
>> when I exit zsh. There's not much point in lexing the history at
>> write-out time, is there?
>
> I can't remember what that is, but I've noticed it before and I have a
> vague feeling I looked and decided I wasn't going to look any more.

Okay, that sounds ominous enough for me to drop the idea.

>> I tried playing around with the code a bit. The thing that looks
>> suspicious to me is
>>     if (*ptr != Meta && imeta(*ptr)) {
>> Shouldn't they check ptr[0] and ptr[1] or so? I tried this,
>>     if ((ptr == lineptr || ptr[-1] != Meta) && imeta(ptr[0])) {
>> (in both places) but it didn't improve matters.
>
> No, the problem here isn't a standard one with Tok -> Meta + NonTok.
> It's that there's something that looks like Tok but isn't.  So we
> need to turn it into Meta + NonTok.  That's the second part of the
> test.  However imeta(*ptr) triggers if *ptr is Meta, which isn't what we
> want because in that case it means we've hit a correct Meta + NonTok
> sequence.
>
> One thing I didn't think I needed to do, but may have got wrong,
> is skip the byte after the Meta, i.e.
>
> if (*ptr == Meta)
>     ptr++;
>
> (which acts together with the existing increment).
>
> You could try that.
>
>> I tried the following instead of the for+if:
>>     unmetafy(lineptr, NULL);
>>     lineptr = metafy(lineptr, -1, META_USEHEAP);
>> and it gets rid of the errors, but it also causes insert-last-word to
>> do nothing. So maybe my whole theory is wrong.
>
> I'd be worried about doing that on every single line -- we already know
> HIST_LEX_WORDS can be very slow, which is the only reason it's an option
> (it's logically the right thing to do, modulo lex failures).
>
> However, it should be possible to combine that with the "remeta" check,
> i.e. see if the line needs it first, but not use the value of remeta,
> just whether it's non-zero, and then it becomes easy and not too
> intrusive --- and also safe about false positives.
>
> I don't see why this would cause failures later on, particularly ones
> apparently unrelated to the meta state of the string.

I figured out that when we assign lineptr after fiddling with it, we
also need to update start, it records the location of *lineptr on
entry to the function, and is used to calculate things later on. With
that addition, the unmetafy + metafy mostly works. insert-last-word
still gets "stuck" on words that came from the old metafication and
starts over from the end of history, leaving the old word in place.

Indeed fc -l lists this line differently still, even though it appears
correctly in zle.
10673* echo mp3info 好きになり\M-c\M-^Aい.mp3
10675* echo mp3info 好きになりたい.mp3

This is with the following code, which fixes(?) all the errors and
seems to make things work pretty well. However, even doing the remeta
unconditionally, I still get the above result with fc -l and
insert-last-word. It also happens, unsurprisingly, if I don't use
hist_lex_words. If I accept a line recalled in this way, the new
history entry is saved correctly and works fine in new shells.

    for (ptr = lineptr; *ptr; ptr++)
        if (*ptr != Meta && imeta(*ptr)) {
            remeta = 1;
            break;
        }
    if (remeta) {
        unmetafy(lineptr, &remeta);
        start = lineptr = metafy(lineptr, remeta, META_USEHEAP);
    }


-- 
Mikael Magnusson


  reply	other threads:[~2015-02-23 11:36 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-02-19 10:13 Peter Stephenson
2015-02-19 21:47 ` Mikael Magnusson
2015-02-19 22:03   ` Peter Stephenson
2015-02-20  3:16     ` Mikael Magnusson
2015-02-20  3:22       ` Mikael Magnusson
2015-02-20  3:33         ` Mikael Magnusson
2015-02-20  3:43           ` Mikael Magnusson
2015-02-20  4:19             ` Ray Andrews
2015-02-20  9:54               ` Peter Stephenson
2015-02-20 10:00             ` Peter Stephenson
2015-02-20 10:12               ` Mikael Magnusson
2015-02-22 18:26                 ` Peter Stephenson
2015-02-23  9:54                   ` Peter Stephenson
2015-02-23 10:11                     ` Peter Stephenson
2015-02-23 11:35                       ` Mikael Magnusson [this message]
2015-02-23 12:36                         ` Peter Stephenson
2015-02-23 12:57                           ` Peter Stephenson
2015-02-23 13:38                             ` Mikael Magnusson
2015-02-23 13:46                               ` Mikael Magnusson
2015-02-23 13:51                                 ` PATCH: Remeta one frame earlier Mikael Magnusson
2015-02-23 13:58                                   ` Peter Stephenson
2015-02-23 14:05                                   ` Peter Stephenson
2015-02-23 14:32                                     ` Mikael Magnusson
2015-02-23 17:32                                       ` Peter Stephenson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAHYJk3R-K=WX=L4qPD207wZaxoE1K-gGxbaywujhTjzXv0fe9w@mail.gmail.com' \
    --to=mikachu@gmail.com \
    --cc=p.stephenson@samsung.com \
    --cc=zsh-workers@zsh.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/zsh/

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 NNTP newsgroup(s).