simon-git: putty (master): Simon Tatham

[Commits to Tartarus hosted VCS]

TL;DR:
  399603fd Counterbodge a bodgy warning from 'aclocal'.
  117c7857 term_bidi_line: fix failure to initialise wcTo.
  209dd65e Rename term->bidi and term->arabicshaping.

Repository:     https://git.tartarus.org/simon/putty.git
On the web:     https://git.tartarus.org/?p=simon/putty.git
Branch updated: master
Committer:      Simon Tatham <anakin at pobox.com>
Date:           2019-03-26 21:31:25

commit 399603fd952aa74793a59e27edb3867b8e7dceb8
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=399603fd952aa74793a59e27edb3867b8e7dceb8;hp=f5c1753244d0399764fd68d6ac42ea2f2db47fc9
Author: Simon Tatham <anakin at pobox.com>
Date:   Tue Mar 26 20:59:31 2019 +0000

    Counterbodge a bodgy warning from 'aclocal'.
    
    For ages, when rebuilding the configure script, I've had the
    mysterious warning "configure.ac:120: warning: macro 'AM_PATH_GTK' not
    found in library". The reason it was mysterious was that that use of
    AM_PATH_GTK was inside an ifdef that checked whether it was defined,
    and it actually wasn't being run!
    
    Turns out the warning comes from 'aclocal', which is a Perl script
    that (among other things) does bodgy text-matching to detect
    unsupported autoconf/automake macros in your configure script. The
    warning comes from the function scan_configure_dep() in that file, as
    of aclocal-1.15. And, indeed, it ignores the ifdef structure, so it
    doesn't notice that I've carefully guarded the use of that possibly-
    undefined macro! (Though a comment in aclocal does acknowledge the
    possibility, which is why it's only a warning.)
    
    Against that bodgy and unreliable check, I've deployed an equally
    bodgy and unreliable countermeasure, by changing the line spacing so
    that AM_PATH_GTK appears in a context (specifically, not immediately
    following a newline or space) where the regex will be confident that
    it's a macro invocation. So that should squelch the warning.

 configure.ac | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

commit 117c7857d2d2706016f624fb36972ca46c16f9c2
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=117c7857d2d2706016f624fb36972ca46c16f9c2;hp=399603fd952aa74793a59e27edb3867b8e7dceb8
Author: Simon Tatham <anakin at pobox.com>
Date:   Tue Mar 26 21:05:12 2019 +0000

    term_bidi_line: fix failure to initialise wcTo.
    
    The bidi algorithm is called on the array term->wcFrom, modifying it
    in place. Then the Arabic-shaping algorithm - which can't work in
    place because it needs to check the original value of array entries
    it's already modified - is called, copying term->wcFrom to term->wcTo
    as a side effect. Then the cleanup code expects the final version of
    the line to be in wcTo. So if shaping is turned off, we still need to
    copy wcFrom into wcTo, even if we don't modify it en route.
    
    Previously, that copy was done under an if statement whose condition
    boils down to 'if bidi is enabled but shaping is not'. So if that code
    was ever reached with _both_ bidi and shaping turned off, then nothing
    at all would copy wcFrom into wcTo, and wcTo would be filled with
    nonsense.
    
    Before trust sigils were introduced, that was OK, because the whole
    function body was skipped if both bidi and shaping were turned off.
    But now trust-sigil handling lives in there too, so we can get into
    that code with the previously disallowed combination of flags. If
    you're lucky, this means that the assert(opos == term->cols) near the
    bottom of the function fails, on the basis that opos is the sum of
    nonsense values from wcTo; if you're unlucky I suppose you might
    manage to get _plausible_ nonsense through to the screen.
    
    Now fixed, by changing that central if statement into a much more
    obvious one: if we're running do_shape, then that can copy wcFrom into
    wcTo, and if and only if we're _not_, then we must copy it another
    way. (And while I'm here, I've turned that other way from a manual for
    loop into memcpy.)

 terminal.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

commit 209dd65ead5fef0d242f92acf96c0a7d8d184402
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=209dd65ead5fef0d242f92acf96c0a7d8d184402;hp=117c7857d2d2706016f624fb36972ca46c16f9c2
Author: Simon Tatham <anakin at pobox.com>
Date:   Tue Mar 26 21:13:19 2019 +0000

    Rename term->bidi and term->arabicshaping.
    
    Those two flags had the opposite sense to what you might expect: each
    one is the value of the Conf entry corresponding to the checkbox that
    _disables_ the corresponding terminal feature. So term->bidi is true
    if and only if bidi is _off_.
    
    I think that confusion of naming probably contributed to the control-
    flow error fixed in the previous commit, just by increasing cognitive
    load until I couldn't remember which flags were set where any more! So
    now I've renamed the two fields of Terminal, and the corresponding
    Conf keywords, to be called "no_bidi" and "no_arabicshaping", in line
    with other 'disable this feature' flags, so that it's clear what the
    sense should be.

 config.c   |  4 ++--
 putty.h    |  4 ++--
 settings.c |  8 ++++----
 terminal.c | 18 +++++++++---------
 terminal.h |  4 ++--
 5 files changed, 19 insertions(+), 19 deletions(-)