Add support for solid cursor blinking style

[skyline75489]

Summary of the Pull Request

Added cursor blinking style. Mainly for solid cursor blinking style which simply disables binking.

References

PR Checklist

• Closes Add support for turning off cursor blink #1379
• CLA signed. If not, go over here and sign the CLA
• Tests added/passed
• Requires documentation to be updated
• I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan.

Detailed Description of the Pull Request / Additional comments

The idea is from #1379 and VSCode. VSCode has several cursor blinking style, including blink, solid, and other animated styles.

Validation Steps Performed

Add configurationi to profile and see how it looks like:

{
    "cursorShape": "filledBox",
    "cursorBlinking": "solid",
}

[Kagami]

👍 cursor blinking is annoying

[miniksa]

I'm not sure about this.

I would almost think what we'd do is instead let you set your own blink time as a setting. If it's null, we call GetCaretBlinkTime() and if it is a value, we use it instead. Then someone can set that value to INFINITE (or we can overload and parse the word 'infinite' and substitute in 0xFFFFFFFF). and just use the existing code.

I would want to hear what

1. @zadjii-msft and @DHowett-MSFT think about having yet another way of modifying the way cursor blink state works
2. @cinnamon-msft on what she thinks is the least confusing way to describe such a configuration option.

[skyline75489]

@miniksa Originally I had the exact idea just like what you described. Then I opened VSCode to see how it handles cursor options and it got me here. VSCode has Cursor Blinking(controls the cursor animation style) and Cursor Style(controls the cursor style), which is two different aspects that coordinate with each other. Even though WT may not need all those fancy cursor animation like VSCode, it provides flexibility to do it. I thought it was great and stole the idea to craft this PR.

[skyline75489]

A side note, VSCode doesn't seem to respect GetCaretBlinkTime() though. Reasonable to me as it is, after all, an editor instead of a terminal emulator.

[j4james]

Another option to consider is just hooking into the Cursor::SetBlinkingAllowed method, the same way the cursorShape, cursorColor, and cursorHeight properties are forwarded to the Cursor::SetStyle method. That way the behavior of the various cursor properties is consistent, and you don't need to reimplement the whole mechanism for turning the blinking on and off - it should just work with the existing code.

Have a look at the way Terminal::UpdateSettings handles the cursor properties here:

terminal/src/cascadia/TerminalCore/Terminal.cpp

Lines 127 to 129 in 60b94a4

	_buffer->GetCursor().SetStyle(settings.CursorHeight(),
	settings.CursorColor(),
	cursorShape);

[qqkookie]

Blinking is only binary on/off option, so how about merging cursor blinking attribute and cursor shape attribute into one attribute as "cursorStyle", like "filledbox" vs "static-filledbox", or "blinking-filledbox" vs "filledbox"?

The name "solid" for non-blinking cursor is confusing. Normally "solid box" means "filledbox". So "solid-emptybox" may be confusing. I prefer "static" as non-blinking attribute name.

The cursor name "filledbox" is rather strange too. I suggest to use name like "solidbox" or "fullbox" (Unicode name) for current "filledbox" attribute value name.

The cursor name "vintage" is also not intuitive. How about " quarterblock"?

Most people will use just one of "static-solidbox" or "blinking-bar". IMHO, default should be static and solidbox (filledbox). And if WT window lose input focus, cursor should stop blinking and turn into static-emptybox.

[qqkookie]

And there is feature request for width of vertical bar and height of filledbox (#3167). These are only applied or meaningful to specific shape of cursor. Having separate attributes for all these may be overly complicated. To prevent such proliferation of cursor attributes and their combinations, cursor style attribute should be limited to single attribute "cursorStyle". For example, "double-bar", "tall-solidbox" with limited variation of blinking and non-blinking.

[3dWrecker]

OK, What if the text and cursor colour are the same? I think we need an inverted colour option.

Or maybe as in Ubuntu:

This exchanges the text colour and background colour in the "characterspace".

[skyline75489]

@3dWrecker Lucky you. That was fixed recently by #6337 . Well not exactly fixed, but it's way better now.

[skyline75489]

This PR is stale and no longer valid. Close it for now.

[mdtauk]

Could there be an option to animate the blinking, by fading it in and out?

[skyline75489]

@mdtauk Let's take the discussion to #1379 where many ideas are shared by people. This PR is way too stale for any meaningful discussion.