Re: tab completion color inversion

Re: tab completion color inversion

[Bart Schaefer]

[>workers]

On Jul 7,  4:49pm, Oliver Kiddle wrote:
}
} The patch below resets the saved colour first but I'd actually be
} inclined to change the following macro to do the AND before the OR.
} 
} #define txtchangeset(T, X, Y)diff((void)(T && (*T |= (X), *T &= ~(Y))))
} 
} It might also then be logical to swap the X and Y arguments.

Reactions:

(1) Is this related to "End boldface also ends background color" thread
from back in March?

(2) As I said in that thread, I don't really follow the semantics of
the txtchangep pointer, but AND before OR seems sensible in the macro.

(3) Swapping the arguments would touch a lot of code and make it hard
to compare revisions, so the benefits ought to be carefully weighed.

Re: tab completion color inversion

[Oliver Kiddle]

Bart wrote:
> Reactions:
> 
> (1) Is this related to "End boldface also ends background color" thread
> from back in March?

Somewhat related. You can get the same, completion altering attributes
effect, with PROMPT='%S%U%Bhello%b '
I see that was never addressed. The code clearly originally had handling
to reset attributes. The TSC_DIRTY flag is included for escape sequences
that apparently reset other attributes. That includes standout off,
underline end, bold on and bold off. If standout end is dirty, you
have to wonder about the bold off sequence that lacks a termcap entry
(\e[22m). Any terminal I have access has no such issues.

The following patch addresses this by restoring colours in addition to
other attributes when TSC_DIRTY is set. There's still an issue after
completion where a final space character isn't redrawn in standout.
And completion is still sometimes wiping attributes and returning to
defaults.

> (2) As I said in that thread, I don't really follow the semantics of
> the txtchangep pointer, but AND before OR seems sensible in the macro.

txtchangep and txtattrmask seem to be doing much the same thing -
tracking the current attributes. The need for tracking attributes that
have been turned off such as TXTNOBOLDFACE seems a bit odd - wouldn't
the absence of TXTBOLDFACE do the job?

> (3) Swapping the arguments would touch a lot of code and make it hard
> to compare revisions, so the benefits ought to be carefully weighed.

I've not done that then but I did put the AND before the OR.

Oliver

diff --git a/Src/Zle/zle_refresh.c b/Src/Zle/zle_refresh.c
index aca676a..e78f1e5 100644
--- a/Src/Zle/zle_refresh.c
+++ b/Src/Zle/zle_refresh.c
@@ -1143,8 +1143,7 @@ zrefresh(void)
 	tsetcap(TCALLATTRSOFF, 0);
 	tsetcap(TCSTANDOUTEND, 0);
 	tsetcap(TCUNDERLINEEND, 0);
-	/* cheat on attribute unset */
-	txtunset(TXTBOLDFACE|TXTSTANDOUT|TXTUNDERLINE);
+	txtattrmask = 0;
 
 	if (trashedzle && !clearflag)
 	    reexpandprompt(); 
diff --git a/Src/prompt.c b/Src/prompt.c
index 831c4f9..bb27453 100644
--- a/Src/prompt.c
+++ b/Src/prompt.c
@@ -523,8 +523,6 @@ putpromptchar(int doprint, int endchar, unsigned int *txtchangep)
 		break;
 	    case 'b':
 		txtchangeset(txtchangep, TXTNOBOLDFACE, TXTBOLDFACE);
-		txtchangeset(txtchangep, TXTNOSTANDOUT, TXTSTANDOUT);
-		txtchangeset(txtchangep, TXTNOUNDERLINE, TXTUNDERLINE);
 		txtunset(TXTBOLDFACE);
 		tsetcap(TCALLATTRSOFF, TSC_PROMPT|TSC_DIRTY);
 		break;
@@ -542,7 +540,8 @@ putpromptchar(int doprint, int endchar, unsigned int *txtchangep)
 		arg = parsecolorchar(arg, 1);
 		if (arg >= 0 && !(arg & TXTNOFGCOLOUR)) {
 		    txtchangeset(txtchangep, arg & TXT_ATTR_FG_ON_MASK,
-				 TXTNOFGCOLOUR);
+				 TXTNOFGCOLOUR | TXT_ATTR_FG_COL_MASK);
+		    txtunset(TXT_ATTR_FG_COL_MASK);
 		    txtset(arg & TXT_ATTR_FG_ON_MASK);
 		    set_colour_attribute(arg, COL_SEQ_FG, TSC_PROMPT);
 		    break;
@@ -557,7 +556,8 @@ putpromptchar(int doprint, int endchar, unsigned int *txtchangep)
 		arg = parsecolorchar(arg, 0);
 		if (arg >= 0 && !(arg & TXTNOBGCOLOUR)) {
 		    txtchangeset(txtchangep, arg & TXT_ATTR_BG_ON_MASK,
-				 TXTNOBGCOLOUR);
+				 TXTNOBGCOLOUR | TXT_ATTR_BG_COL_MASK);
+		    txtunset(TXT_ATTR_BG_COL_MASK);
 		    txtset(arg & TXT_ATTR_BG_ON_MASK);
 		    set_colour_attribute(arg, COL_SEQ_BG, TSC_PROMPT);
 		    break;
@@ -1041,6 +1041,10 @@ tsetcap(int cap, int flags)
 		tsetcap(TCSTANDOUTBEG, flags);
 	    if (txtisset(TXTUNDERLINE))
 		tsetcap(TCUNDERLINEBEG, flags);
+	    if (txtisset(TXTFGCOLOUR))
+		set_colour_attribute(txtattrmask, COL_SEQ_FG, TSC_PROMPT);
+	    if (txtisset(TXTBGCOLOUR))
+		set_colour_attribute(txtattrmask, COL_SEQ_BG, TSC_PROMPT);
 	}
     }
 }
diff --git a/Src/zsh.h b/Src/zsh.h
index eee31da..36fddd0 100644
--- a/Src/zsh.h
+++ b/Src/zsh.h
@@ -2567,7 +2567,7 @@ struct ttyinfo {
 
 #define txtchangeisset(T,X)	((T) & (X))
 #define txtchangeget(T,A)	(((T) & A ## _MASK) >> A ## _SHIFT)
-#define txtchangeset(T, X, Y)	((void)(T && (*T |= (X), *T &= ~(Y))))
+#define txtchangeset(T, X, Y)	((void)(T && (*T &= ~(Y), *T |= (X))))
 
 /*
  * For outputting sequences to change colour: specify foreground

Re: tab completion color inversion

[Bart Schaefer]

On Jul 8,  1:21am, Oliver Kiddle wrote:
}
} You can get the same, completion altering attributes
} effect, with PROMPT='%S%U%Bhello%b '

Presumably it's intentional that reaching the end of the prompt string
does not implicitly reset all the %S %U etc. attributes?  E.g., meant
to be combined with ( preexec() { print -nP %u%s } ) or similar.

Using that prompt I can still get effects where the %S%U ends in the
middle of a command line after completing.  I don't think this is a
bug, per se, because leaving the attributes hanging open like that is
not exactly documented behavior, but it might argue that they should
be implicitly turned off.

(I think completion always resets the attributes, but sometimes ZLE
reprints the whole prompt which turns them back on again.)

} txtchangep and txtattrmask seem to be doing much the same thing -
} tracking the current attributes. The need for tracking attributes that
} have been turned off such as TXTNOBOLDFACE seems a bit odd - wouldn't
} the absence of TXTBOLDFACE do the job?

You've stated most of my confusion over this, quite succinctly.  My
only guess is that one represents a temporary state change from the
more persistent state of the other, but I haven't worked out how that
plays or which is which.

} I've not done that then but I did put the AND before the OR.

In thinking about this further, swapping the AND and OR subexpressions
does change the semantics when the same bit is present in both X and Y.
Maybe that never comes up.

Re: tab completion color inversion

[Oliver Kiddle]

Bart wrote:
> Presumably it's intentional that reaching the end of the prompt string
> does not implicitly reset all the %S %U etc. attributes?  E.g., meant
> to be combined with ( preexec() { print -nP %u%s } ) or similar.

This was never done in the past. And until we had zle_highlight it
was necessary if you wanted different attributes for the text you
entered and for command output. Completion has spoiled that concept
since long ago, however:

> Using that prompt I can still get effects where the %S%U ends in the
> middle of a command line after completing.  I don't think this is a
> bug, per se, because leaving the attributes hanging open like that is
> not exactly documented behavior, but it might argue that they should
> be implicitly turned off.

We should probably treat it as a bug if we can track it down. For
backward compatibility, I'm inclined to think that whatever is
leftover after PS1 should be used as a default if zle_highlight
doesn't give something more explicit. Using zle_highlight does seem
to work together with completion.

Note also that txtattrmask is not reset between invocations of print -P
which is useful in some cases but not in others. Try this:
  print -P '%S' >/dev/null; print -P '%bhello'

> (I think completion always resets the attributes, but sometimes ZLE
> reprints the whole prompt which turns them back on again.)

Completion sometimes restores the attributes and sometimes resets them.

> } txtchangep and txtattrmask seem to be doing much the same thing -
> } tracking the current attributes. The need for tracking attributes that
> } have been turned off such as TXTNOBOLDFACE seems a bit odd - wouldn't
> } the absence of TXTBOLDFACE do the job?
>
> You've stated most of my confusion over this, quite succinctly.  My
> only guess is that one represents a temporary state change from the
> more persistent state of the other, but I haven't worked out how that
> plays or which is which.

Yes, TXTNOBOLDFACE only makes sense if recording relative attribute
changes. Quite how that's useful, I'm not sure. If attributes are
tracked, a simple XOR with desired attributes gets you what needs
changing.

> } I've not done that then but I did put the AND before the OR.
>
> In thinking about this further, swapping the AND and OR subexpressions
> does change the semantics when the same bit is present in both X and Y.
> Maybe that never comes up.

It doesn't. I did grep for it and that case only came up for the colours
which is where we want AND first.

Oliver