Re: PATCH: several bug fixes in countprompt

[Roman Perepelitsa <roman.perepelitsa@xxxxxxxxx>]

On Wed, Jun 19, 2019 at 12:56 PM Peter Stephenson
<p.stephenson@xxxxxxxxxxx> wrote:
>
> On Wed, 2019-06-19 at 12:22 +0200, Roman Perepelitsa wrote:
> > This patch fixes multiple bugs in countprompt.
> >...
> > I've only tested it on one terminal. Before I test it on a dozen of
> > different terminals, is there anything I'm missing? Anything I should
> > be on lookout for?
>
> Thanks.  I think the experience has usually been wide characters either
> work on a terminal or don't work, but I don't think anybody's specifcally
> looked for edge cases before.  Other characters and effects are usually
> fairly predictable.

Got it.

I've tested the patch on several terminals. Here's what I've found.

1. All terminals suffer from bug #1 and the patch fixes it everywhere.

2. All terminals that support wide characters suffer from bug #2 and
the patch fixes it. Terminals that don't support wide characters have
incorrect cursor position both with and without my patch.

3. All terminals suffer from bug #3. However, my patch fixes it only
for some terminals but not for all. There are two ways terminals
handle tabs that don't fit on the line. The first is what I've
described in the patch: tabs are treated as sequences of spaces, and
some spaces can overflow to the next line. Thus, with COLUMNS=77 and
PROMPT consisting of 10 tabs, prompt ends up taking two lines with the
second line having just 3 spaces on it. The second way is that the
cursor remains on the last column of the first line without ever
wrapping around to the next line. In addition, cursor behaves rather
unexpectedly when you start typing in such prompt. The first typed
character appears at the end of the line, the second typed character
also appears at the end of the line while pushing the previous
character to the left, but the third character overflows to the next
line.

To summarize, my patch doesn't seem to break anything, fixes bug #1,
fixes bug #2 everywhere where it makes sense, and fixes bug #3 where
it's feasible.

Roman.