xterm.js
xterm.js copied to clipboard
ITerminalOptions: Make fields required
In #3667, all getOptions were replaced by their new equivalent:
this.dimensions.scaledCharLeft = Math.floor(this._terminal.getOption('letterSpacing') / 2);
became
this.dimensions.scaledCharLeft = Math.floor(this._terminal.options.letterSpacing! / 2);
Mind the "!" there, which is a shortcut to let typescript know that the variable is for sure defined. In fact, that can be safely assumed since the default parameters are filled in.
If the class ITerminalOptions is changed to have required fields which are not optional, it would be possible to have
this.dimensions.scaledCharLeft = Math.floor(this._terminal.options.letterSpacing / 2);
It was kind of explored in #3448 that
options: ITerminalOptions;
can be changed to
get options(): ITerminalOptions;
set options(options: Partial<ITerminalOptions>): void;
which would be an equivalent of the current code. Just ITerminalOptions is changed to have required fields.
There are some settings though which can be "undefined":
- fastScrollModifier: Perhaps we can introduce "none" here
- overviewRulerWidth: Use 0 to have this disabled?
This would also make the code more aligned, since the typings have ITerminalOptions with all fields optional and the internal ITerminalOptions have required fields.
The breaking change part here lies in the fact that addons can use ITerminalOptions as type. If this is passed as a parameter, they would need to change the parameter to Partial<ITerminalOptions>.
I like it. Also yes we should get rid of undefined in fastScrollModifier and overviewRulerWidth as I'm not even sure it's possible to unset them in the current state, I like your proposals.
Let's do this as part of v5 so we can make the (minor) breaking change
Let me know if you don't have time in the next week and I can pick this up