fix: in minisplits, blocks that contain pty output may have excess whitespace

This PR adds a way for streaming controllers to request that the view remove all prior output. Previously, the pty/client controller was calling xtermContainer.remove() directly, i.e. it was manipulating the DOM directly. Nope!!

Fixes #6759

Author: starpit

plugins/plugin-bash-like/src/pty/client.ts

@@ -612,7 +612,10 @@ async function initOnMessage(
         //     note: this is still an open issue for things like `git log` or `less` where
         //           the xterm stays alive in race with scrolling
         window.requestAnimationFrame(() => {
-          xtermContainer.remove()
+          // this tells the view to clear any prior output
+          // !! DO NOT CALL xtermContainer.remove() directly !!
+          execOptions.stdout(null)
+
           // respond to the REPL
           respondToRepl({
             apiVersion: 'kui-shell/v1',
@@ -883,7 +886,9 @@ export const doExec = (
               (execOptions.type === ExecType.Nested && execOptions.quiet !== false) ||
               resizer.wasEverInAltBufferMode()
             ) {
-              xtermContainer.remove()
+              // this tells the view to clear any prior output
+              // !! DO NOT CALL xtermContainer.remove() directly !!
+              execOptions.stdout(null)
             } else {
               xtermContainer.classList.add('xterm-terminated')
             }

plugins/plugin-client-common/src/components/Content/Scalar/HTMLDom.tsx

@@ -49,15 +49,17 @@ export default class HTMLDom extends React.PureComponent<Props> {
   }
 
   public render() {
-    return (
+    return !this.props.content ? (
+      <React.Fragment />
+    ) : (
       <div
         className={
           'padding-content scrollable scrollable-auto page-content' +
           (this.props.className ? ` ${this.props.className}` : '')
         }
         style={{ display: 'flex', flex: 1 }}
       >
-        <div style={{ display: 'flex', flex: 1 }} ref={dom => this.setState({ dom })} />
+        <div className="kui--ignore-if-empty" ref={dom => this.setState({ dom })} />
       </div>
     )
   }

plugins/plugin-client-common/src/components/Content/Scalar/XtermDom.tsx

@@ -47,7 +47,9 @@ export default class XtermDom extends React.PureComponent<Props> {
   }
 
   public render() {
-    return (
+    return this.props.response.rows.length === 0 ? (
+      <React.Fragment />
+    ) : (
       <div className="padding-content scrollable scrollable-auto page-content" style={{ display: 'flex', flex: 1 }}>
         <div style={{ display: 'flex', flex: 1 }}>
           <div className="xterm-container xterm-terminated">

plugins/plugin-client-common/src/components/Views/Terminal/Block/Output.tsx

@@ -115,8 +115,9 @@ export default class Output extends React.PureComponent<Props, State> {
   // eslint-disable-next-line @typescript-eslint/no-unused-vars
   private async streamingConsumer(part: Streamable) {
     if (hasUUID(this.props.model)) {
+      // part === null: the controller wants to clear any prior output
       this.setState(curState => ({
-        streamingOutput: curState.streamingOutput.concat([part])
+        streamingOutput: part === null ? [] : curState.streamingOutput.concat([part])
       }))
       this.props.onRender()
       eventChannelUnsafe.emit(`/command/stdout/done/${this.props.uuid}/${this.props.model.execUUID}`)

plugins/plugin-client-common/web/css/static/ui.css

@@ -127,6 +127,13 @@ body.still-loading .repl {
 }
 
 /* generic */
+.kui--ignore-if-empty {
+  display: flex;
+  flex: 1;
+  &:empty {
+    display: none;
+  }
+}
 .kui--hero-text {
   flex: 1;
   display: flex;