Repository: microsoft/terminal (language: cpp)

Issue:
Do we really need the DispatchCommon class?
# Description of the new feature/enhancement

If we move the `DispatchCommon::s_ResizeWindow` implementation into the `ConhostInternalGetSet` class, then `DispatchCommon` is just a set of static functions that indirectly call into the `ConGetSet` interface. The calling code could just as easily call the `ConGetSet` API directly.

# Proposed technical implementation details (optional)

Everywhere that we're making a call like this:

    DispatchCommon::s_RefreshWindow(*_pConApi);

we'd simple change it to something like this:

    _pConApi->RefreshWindow();



Discussion / hints:

Fix patch:
diff --git a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp
index ec2ef86d12a..8a812d75a2e 100644
--- a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp
+++ b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp
@@ -292,7 +292,7 @@ void ConptyRoundtripTests::_resizeConpty(const unsigned short sx,
                                          const unsigned short sy)
 {
     // Largely taken from implementation in PtySignalInputThread::_InputThread
-    if (DispatchCommon::s_ResizeWindow(*_pConApi, sx, sy))
+    if (_pConApi->ResizeWindow(sx, sy))
     {
         auto& g = ServiceLocator::LocateGlobals();
         auto& gci = g.getConsoleInformation();
diff --git a/src/host/PtySignalInputThread.cpp b/src/host/PtySignalInputThread.cpp
index 2873fd2a311..d480e135617 100644
--- a/src/host/PtySignalInputThread.cpp
+++ b/src/host/PtySignalInputThread.cpp
@@ -8,7 +8,6 @@
 #include "output.h"
 #include "handle.h"
 #include "../interactivity/inc/ServiceLocator.hpp"
-#include "../terminal/adapter/DispatchCommon.hpp"
 
 using namespace Microsoft::Console;
 using namespace Microsoft::Console::Interactivity;
@@ -137,9 +136,9 @@ void PtySignalInputThread::ConnectConsole() noexcept
 // - <none>
 void PtySignalInputThread::_DoResizeWindow(const ResizeWindowData& data)
 {
-    if (DispatchCommon::s_ResizeWindow(*_pConApi, data.sx, data.sy))
+    if (_pConApi->ResizeWindow(data.sx, data.sy))
     {
-        DispatchCommon::s_SuppressResizeRepaint(*_pConApi);
+        _pConApi->SuppressResizeRepaint();
     }
 }
 
diff --git a/src/host/outputStream.cpp b/src/host/outputStream.cpp
index f82eefe67fb..08a8c8020ba 100644
--- a/src/host/outputStream.cpp
+++ b/src/host/outputStream.cpp
@@ -608,6 +608,50 @@ unsigned int ConhostInternalGetSet::GetConsoleOutputCP() const
     return ServiceLocator::LocateGlobals().getConsoleInformation().OutputCP;
 }
 
+// Routine Description:
+// - Resizes the window to the specified dimensions, in characters.
+// Arguments:
+// - width: The new width of the window, in columns
+// - height: The new height of the window, in rows
+// Return Value:
+// - True if handled successfully. False otherwise.
+bool ConhostInternalGetSet::ResizeWindow(const size_t width, const size_t height)
+{
+    SHORT sColumns = 0;
+    SHORT sRows = 0;
+
+    THROW_IF_FAILED(SizeTToShort(width, &sColumns));
+    THROW_IF_FAILED(SizeTToShort(height, &sRows));
+    // We should do nothing if 0 is passed in for a size.
+    RETURN_BOOL_IF_FALSE(width > 0 && height > 0);
+
+    CONSOLE_SCREEN_BUFFER_INFOEX csbiex = { 0 };
+    csbiex.cbSize = sizeof(CONSOLE_SCREEN_BUFFER_INFOEX);
+    GetConsoleScreenBufferInfoEx(csbiex);
+
+    const Viewport oldViewport = Viewport::FromInclusive(csbiex.srWindow);
+    const Viewport newViewport = Viewport::FromDimensions(oldViewport.Origin(), sColumns, sRows);
+    // Always resize the width of the console
+    csbiex.dwSize.X = sColumns;
+    // Only set the screen buffer's height if it's currently less than
+    //  what we're requesting.
+    if (sRows > csbiex.dwSize.Y)
+    {
+        csbiex.dwSize.Y = sRows;
+    }
+
+    // SetWindowInfo expect inclusive rects
+    const auto sri = newViewport.ToInclusive();
+
+    // SetConsoleScreenBufferInfoEx however expects exclusive rects
+    const auto sre = newViewport.ToExclusive();
+    csbiex.srWindow = sre;
+
+    SetConsoleScreenBufferInfoEx(csbiex);
+    SetWindowInfo(true, sri);
+    return true;
+}
+
 // Routine Description:
 // - Forces the VT Renderer to NOT paint the next resize event. This is used by
 //      InteractDispatch, to prevent resizes from echoing between terminal and host.
diff --git a/src/host/outputStream.hpp b/src/host/outputStream.hpp
index 53745669622..581730cf914 100644
--- a/src/host/outputStream.hpp
+++ b/src/host/outputStream.hpp
@@ -101,7 +101,7 @@ class ConhostInternalGetSet final : public Microsoft::Console::VirtualTerminal::
     void SetCursorStyle(CursorType const style) override;
 
     void RefreshWindow() override;
-
+    bool ResizeWindow(const size_t width, const size_t height) override;
     void SuppressResizeRepaint() override;
 
     void WriteControlInput(const KeyEvent key) override;
diff --git a/src/terminal/adapter/DispatchCommon.cpp b/src/terminal/adapter/DispatchCommon.cpp
deleted file mode 100644
index 4a00590cb8c..00000000000
--- a/src/terminal/adapter/DispatchCommon.cpp
+++ /dev/null
@@ -1,82 +0,0 @@
-// Copyright (c) Microsoft Corporation.
-// Licensed under the MIT license.
-
-#include "precomp.h"
-
-#include "DispatchCommon.hpp"
-#include "../../types/inc/Viewport.hpp"
-
-using namespace Microsoft::Console::Types;
-using namespace Microsoft::Console::VirtualTerminal;
-
-// Method Description:
-// - Resizes the window to the specified dimensions, in characters.
-// Arguments:
-// - conApi: The ConGetSet implementation to call back into.
-// - width: The new width of the window, in columns
-// - height: The new height of the window, in rows
-// Return Value:
-// True if handled successfully. False otherwise.
-bool DispatchCommon::s_ResizeWindow(ConGetSet& conApi,
-                                    const size_t width,
-                                    const size_t height)
-{
-    SHORT sColumns = 0;
-    SHORT sRows = 0;
-
-    THROW_IF_FAILED(SizeTToShort(width, &sColumns));
-    THROW_IF_FAILED(SizeTToShort(height, &sRows));
-    // We should do nothing if 0 is passed in for a size.
-    RETURN_BOOL_IF_FALSE(width > 0 && height > 0);
-
-    CONSOLE_SCREEN_BUFFER_INFOEX csbiex = { 0 };
-    csbiex.cbSize = sizeof(CONSOLE_SCREEN_BUFFER_INFOEX);
-    conApi.GetConsoleScreenBufferInfoEx(csbiex);
-
-    const Viewport oldViewport = Viewport::FromInclusive(csbiex.srWindow);
-    const Viewport newViewport = Viewport::FromDimensions(oldViewport.Origin(), sColumns, sRows);
-    // Always resize the width of the console
-    csbiex.dwSize.X = sColumns;
-    // Only set the screen buffer's height if it's currently less than
-    //  what we're requesting.
-    if (sRows > csbiex.dwSize.Y)
-    {
-        csbiex.dwSize.Y = sRows;
-    }
-
-    // SetWindowInfo expect inclusive rects
-    const auto sri = newViewport.ToInclusive();
-
-    // SetConsoleScreenBufferInfoEx however expects exclusive rects
-    const auto sre = newViewport.ToExclusive();
-    csbiex.srWindow = sre;
-
-    conApi.SetConsoleScreenBufferInfoEx(csbiex);
-    conApi.SetWindowInfo(true, sri);
-    return true;
-}
-
-// Routine Description:
-// - Force the host to repaint the screen.
-// Arguments:
-// - conApi: The ConGetSet implementation to call back into.
-// Return Value:
-// - <none>
-void DispatchCommon::s_RefreshWindow(ConGetSet& conApi)
-{
-    conApi.RefreshWindow();
-}
-
-// Routine Description:
-// - Force the host to tell the renderer to not emit anything in response to the
-//      next resize event. This is used by VT I/O to prevent a terminal from
-//      requesting a resize, then having the renderer echo that to the terminal,
-//      then having the terminal echo back to the host...
-// Arguments:
-// - conApi: The ConGetSet implementation to call back into.
-// Return Value:
-// - <none>
-void DispatchCommon::s_SuppressResizeRepaint(ConGetSet& conApi)
-{
-    conApi.SuppressResizeRepaint();
-}
diff --git a/src/terminal/adapter/DispatchCommon.hpp b/src/terminal/adapter/DispatchCommon.hpp
deleted file mode 100644
index 0146d28d378..00000000000
--- a/src/terminal/adapter/DispatchCommon.hpp
+++ /dev/null
@@ -1,33 +0,0 @@
-/*++
-Copyright (c) Microsoft Corporation
-Licensed under the MIT license.
-
-Module Name:
-- DispatchCommon.hpp
-
-Abstract:
-- Defines a number of common functions and enums whose implementation is the
-    same in both the AdaptDispatch and the InteractDispatch.
-
-Author(s):
-- Mike Griese (migrie) 11 Oct 2017
---*/
-
-#pragma once
-
-#include "conGetSet.hpp"
-
-namespace Microsoft::Console::VirtualTerminal
-{
-    class DispatchCommon final
-    {
-    public:
-        static bool s_ResizeWindow(ConGetSet& conApi,
-                                   const size_t width,
-                                   const size_t height);
-
-        static void s_RefreshWindow(ConGetSet& conApi);
-
-        static void s_SuppressResizeRepaint(ConGetSet& conApi);
-    };
-}
diff --git a/src/terminal/adapter/InteractDispatch.cpp b/src/terminal/adapter/InteractDispatch.cpp
index 5145a188eb7..81b7290dc7c 100644
--- a/src/terminal/adapter/InteractDispatch.cpp
+++ b/src/terminal/adapter/InteractDispatch.cpp
@@ -4,7 +4,6 @@
 #include "precomp.h"
 
 #include "InteractDispatch.hpp"
-#include "DispatchCommon.hpp"
 #include "conGetSet.hpp"
 #include "../../interactivity/inc/EventSynthesis.hpp"
 #include "../../types/inc/Viewport.hpp"
@@ -102,14 +101,14 @@ bool InteractDispatch::WindowManipulation(const DispatchTypes::WindowManipulatio
     switch (function)
     {
     case DispatchTypes::WindowManipulationType::RefreshWindow:
-        DispatchCommon::s_RefreshWindow(*_pConApi);
+        _pConApi->RefreshWindow();
         return true;
     case DispatchTypes::WindowManipulationType::ResizeWindowInCharacters:
         // TODO:GH#1765 We should introduce a better `ResizeConpty` function to
         // the ConGetSet interface, that specifically handles a conpty resize.
-        if (DispatchCommon::s_ResizeWindow(*_pConApi, parameter2.value_or(0), parameter1.value_or(0)))
+        if (_pConApi->ResizeWindow(parameter2.value_or(0), parameter1.value_or(0)))
         {
-            DispatchCommon::s_SuppressResizeRepaint(*_pConApi);
+            _pConApi->SuppressResizeRepaint();
         }
         return true;
     default:
diff --git a/src/terminal/adapter/adaptDispatch.cpp b/src/terminal/adapter/adaptDispatch.cpp
index 826233cec84..6599914c5d0 100644
--- a/src/terminal/adapter/adaptDispatch.cpp
+++ b/src/terminal/adapter/adaptDispatch.cpp
@@ -2183,10 +2183,10 @@ bool AdaptDispatch::WindowManipulation(const DispatchTypes::WindowManipulationTy
     switch (function)
     {
     case DispatchTypes::WindowManipulationType::RefreshWindow:
-        DispatchCommon::s_RefreshWindow(*_pConApi);
+        _pConApi->RefreshWindow();
         return true;
     case DispatchTypes::WindowManipulationType::ResizeWindowInCharacters:
-        DispatchCommon::s_ResizeWindow(*_pConApi, parameter2.value_or(0), parameter1.value_or(0));
+        _pConApi->ResizeWindow(parameter2.value_or(0), parameter1.value_or(0));
         return true;
     default:
         return false;
diff --git a/src/terminal/adapter/adaptDispatch.hpp b/src/terminal/adapter/adaptDispatch.hpp
index 97aefa8aebb..fdd4834f2b5 100644
--- a/src/terminal/adapter/adaptDispatch.hpp
+++ b/src/terminal/adapter/adaptDispatch.hpp
@@ -15,7 +15,6 @@ Author(s):
 #pragma once
 
 #include "termDispatch.hpp"
-#include "DispatchCommon.hpp"
 #include "conGetSet.hpp"
 #include "adaptDefaults.hpp"
 #include "FontBuffer.hpp"
diff --git a/src/terminal/adapter/conGetSet.hpp b/src/terminal/adapter/conGetSet.hpp
index 7eb6e865a45..5c9d90b11df 100644
--- a/src/terminal/adapter/conGetSet.hpp
+++ b/src/terminal/adapter/conGetSet.hpp
@@ -79,6 +79,7 @@ namespace Microsoft::Console::VirtualTerminal
         virtual void SetConsoleOutputCP(const unsigned int codepage) = 0;
         virtual unsigned int GetConsoleOutputCP() const = 0;
 
+        virtual bool ResizeWindow(const size_t width, const size_t height) = 0;
         virtual void SuppressResizeRepaint() = 0;
         virtual bool IsConsolePty() const = 0;
 
diff --git a/src/terminal/adapter/lib/adapter.vcxproj b/src/terminal/adapter/lib/adapter.vcxproj
index a2c7b78b3ef..38796917924 100644
--- a/src/terminal/adapter/lib/adapter.vcxproj
+++ b/src/terminal/adapter/lib/adapter.vcxproj
@@ -11,7 +11,6 @@
   <Import Project="$(SolutionDir)src\common.build.pre.props" />
   <ItemGroup>
     <ClCompile Include="..\adaptDispatch.cpp" />
-    <ClCompile Include="..\DispatchCommon.cpp" />
     <ClCompile Include="..\FontBuffer.cpp" />
     <ClCompile Include="..\InteractDispatch.cpp" />
     <ClCompile Include="..\adaptDispatchGraphics.cpp" />
@@ -27,7 +26,6 @@
     <ClInclude Include="..\adaptDispatch.hpp" />
     <ClInclude Include="..\charsets.hpp" />
     <ClInclude Include="..\DispatchTypes.hpp" />
-    <ClInclude Include="..\DispatchCommon.hpp" />
     <ClInclude Include="..\FontBuffer.hpp" />
     <ClInclude Include="..\InteractDispatch.hpp" />
     <ClInclude Include="..\conGetSet.hpp" />
diff --git a/src/terminal/adapter/lib/adapter.vcxproj.filters b/src/terminal/adapter/lib/adapter.vcxproj.filters
index 1d76cc8d648..96f433000d4 100644
--- a/src/terminal/adapter/lib/adapter.vcxproj.filters
+++ b/src/terminal/adapter/lib/adapter.vcxproj.filters
@@ -33,9 +33,6 @@
     <ClCompile Include="..\precomp.cpp">
       <Filter>Source Files</Filter>
     </ClCompile>
-    <ClCompile Include="..\DispatchCommon.cpp">
-      <Filter>Source Files</Filter>
-    </ClCompile>
     <ClCompile Include="..\InteractDispatch.cpp">
       <Filter>Source Files</Filter>
     </ClCompile>
@@ -65,9 +62,6 @@
     <ClInclude Include="..\tracing.hpp">
       <Filter>Header Files</Filter>
     </ClInclude>
-    <ClInclude Include="..\DispatchCommon.hpp">
-      <Filter>Header Files</Filter>
-    </ClInclude>
     <ClInclude Include="..\IInteractDispatch.hpp">
       <Filter>Header Files</Filter>
     </ClInclude>
diff --git a/src/terminal/adapter/sources.inc b/src/terminal/adapter/sources.inc
index 8b3ae4f4339..42639276dfd 100644
--- a/src/terminal/adapter/sources.inc
+++ b/src/terminal/adapter/sources.inc
@@ -31,7 +31,6 @@ PRECOMPILED_INCLUDE     = ..\precomp.h
 
 SOURCES= \
     ..\adaptDispatch.cpp \
-    ..\DispatchCommon.cpp \
     ..\FontBuffer.cpp \
     ..\InteractDispatch.cpp \
     ..\adaptDispatchGraphics.cpp \
diff --git a/src/terminal/adapter/ut_adapter/adapterTest.cpp b/src/terminal/adapter/ut_adapter/adapterTest.cpp
index 9ef4c7aaaf6..97723dff54e 100644
--- a/src/terminal/adapter/ut_adapter/adapterTest.cpp
+++ b/src/terminal/adapter/ut_adapter/adapterTest.cpp
@@ -313,6 +313,12 @@ class TestGetSet final : public ConGetSet
         Log::Comment(L"RefreshWindow MOCK called...");
     }
 
+    bool ResizeWindow(const size_t /*width*/, const size_t /*height*/) override
+    {
+        Log::Comment(L"ResizeWindow MOCK called...");
+        return true;
+    }
+
     void SuppressResizeRepaint() override
     {
         Log::Comment(L"SuppressResizeRepaint MOCK called...");


Test patch: