Repository: microsoft/terminal (language: cpp)

Issue:
Export Text when in Admin mode filename suggestion has invalid character ":"
### Windows Terminal version

1.14.1963.0

### Windows build number

22000.795

### Other Software

_No response_

### Steps to reproduce

Open Windows Terminal (Admin).
Have text in console window.
Right click on tab and choose "Export Text".

### Expected Behavior

Cleanse out the colon character and other invalid characters for filenames based on the tab name.

### Actual Behavior

Observe filename provided has "Admin: " as the leading text, and on Windows, a colon is not valid in filenames.


Discussion / hints:
I mean, I guess why not? We'd probably accept this as a contribution
If @restartedwill is not willing to work on this issue, I would try to tackle this, but I'll need guidance to identify what files need to be examined and edited 
Yes, I am not able to tackle this change; just raising awareness in the hopes another developer could pick it up.

 

From: EliaSchiavon ***@***.***> 
Sent: Friday, August 5, 2022 4:16 AM
To: microsoft/terminal ***@***.***>
Cc: Will DP ***@***.***>; Mention ***@***.***>
Subject: Re: [microsoft/terminal] Export Text when in Admin mode filename suggestion has invalid character ":" (Issue #13664)

 

If @restartedwill <https://github.com/restartedwill>  is not willing to work on this issue, I would try to tackle this, but I'll need guidance to identify what files need to be examined and edited

—
Reply to this email directly, view it on GitHub <https://github.com/microsoft/terminal/issues/13664#issuecomment-1206284826> , or unsubscribe <https://github.com/notifications/unsubscribe-auth/AGHJGH5IQU2AV24YK6GC7RLVXTSWTANCNFSM55POEZIA> .
You are receiving this because you were mentioned.  <https://github.com/notifications/beacon/AGHJGHZN5EOK47B4U2XGEITVXTSWTA5CNFSM55POEZIKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOI7THEGQ.gif> Message ID: ***@***.***>


That'd be great! I'd start right here:

https://github.com/microsoft/terminal/blob/74cdffe9215740a2c783c2d1f393fd303286b2b5/src/cascadia/TerminalApp/TabManagement.cpp#L435-L436


That's the line where we set the proposed text. I don't think we have any existing helpers for trimming a string so that it's a viable filename - I'll leave that as an exercise for the reader 😉 
Ahah thank you! Started working on it, I will raise a PR in the next few days

Fix patch:
diff --git a/src/cascadia/TerminalApp/TabManagement.cpp b/src/cascadia/TerminalApp/TabManagement.cpp
index 97679d99b3c..8ebcc47c034 100644
--- a/src/cascadia/TerminalApp/TabManagement.cpp
+++ b/src/cascadia/TerminalApp/TabManagement.cpp
@@ -12,6 +12,7 @@
 #include "TerminalPage.h"
 #include "Utils.h"
 #include "../../types/inc/utils.hpp"
+#include "../../inc/til/string.h"
 
 #include <LibraryResources.h>
 
@@ -433,7 +434,9 @@ namespace winrt::TerminalApp::implementation
                         THROW_IF_FAILED(dialog->SetDefaultExtension(L"txt"));
 
                         // Default to using the tab title as the file name
-                        THROW_IF_FAILED(dialog->SetFileName((control.Title() + L".txt").c_str()));
+                        std::wstring filename{ control.Title() };
+                        filename = til::clean_filename(filename);
+                        THROW_IF_FAILED(dialog->SetFileName((filename + L".txt").c_str()));
                     });
                 }
                 else
diff --git a/src/inc/til/string.h b/src/inc/til/string.h
index d09eb159b71..f7fb0ae757c 100644
--- a/src/inc/til/string.h
+++ b/src/inc/til/string.h
@@ -30,6 +30,29 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned"
         return visualize_control_codes(std::wstring{ str });
     }
 
+    _TIL_INLINEPREFIX std::wstring clean_filename(std::wstring str) noexcept
+    {
+        static constexpr std::array<bool, 128> filter{ {
+            // clang-format off
+            0 /* NUL */, 0 /* SOH */, 0 /* STX */, 0 /* ETX */, 0 /* EOT */, 0 /* ENQ */, 0 /* ACK */, 0 /* BEL */, 0 /* BS  */, 0 /* HT  */, 0 /* LF  */, 0 /* VT  */, 0 /* FF  */, 0 /* CR  */, 0 /* SO  */, 0 /* SI  */,
+            0 /* DLE */, 0 /* DC1 */, 0 /* DC2 */, 0 /* DC3 */, 0 /* DC4 */, 0 /* NAK */, 0 /* SYN */, 0 /* ETB */, 0 /* CAN */, 0 /* EM  */, 0 /* SUB */, 0 /* ESC */, 0 /* FS  */, 0 /* GS  */, 0 /* RS  */, 0 /* US  */,
+            0 /* SP  */, 0 /* !   */, 1 /* "   */, 0 /* #   */, 0 /* $   */, 0 /* %   */, 0 /* &   */, 0 /* '   */, 0 /* (   */, 0 /* )   */, 1 /* *   */, 0 /* +   */, 0 /* ,   */, 0 /* -   */, 0 /* .   */, 1 /* /   */,
+            0 /* 0   */, 0 /* 1   */, 0 /* 2   */, 0 /* 3   */, 0 /* 4   */, 0 /* 5   */, 0 /* 6   */, 0 /* 7   */, 0 /* 8   */, 0 /* 9   */, 1 /* :   */, 0 /* ;   */, 1 /* <   */, 0 /* =   */, 1 /* >   */, 1 /* ?   */,
+            0 /* @   */, 0 /* A   */, 0 /* B   */, 0 /* C   */, 0 /* D   */, 0 /* E   */, 0 /* F   */, 0 /* G   */, 0 /* H   */, 0 /* I   */, 0 /* J   */, 0 /* K   */, 0 /* L   */, 0 /* M   */, 0 /* N   */, 0 /* O   */,
+            0 /* P   */, 0 /* Q   */, 0 /* R   */, 0 /* S   */, 0 /* T   */, 0 /* U   */, 0 /* V   */, 0 /* W   */, 0 /* X   */, 0 /* Y   */, 0 /* Z   */, 0 /* [   */, 1 /* \   */, 0 /* ]   */, 0 /* ^   */, 0 /* _   */,
+            0 /* `   */, 0 /* a   */, 0 /* b   */, 0 /* c   */, 0 /* d   */, 0 /* e   */, 0 /* f   */, 0 /* g   */, 0 /* h   */, 0 /* i   */, 0 /* j   */, 0 /* k   */, 0 /* l   */, 0 /* m   */, 0 /* n   */, 0 /* o   */,
+            0 /* p   */, 0 /* q   */, 0 /* r   */, 0 /* s   */, 0 /* t   */, 0 /* u   */, 0 /* v   */, 0 /* w   */, 0 /* x   */, 0 /* y   */, 0 /* z   */, 0 /* {   */, 1 /* |   */, 0 /* }   */, 0 /* ~   */, 0 /* DEL */,
+            // clang-format on
+        } };
+
+        std::erase_if(str, [](auto ch) {
+            // This lookup is branchless: It always checks the filter, but throws
+            // away the result if ch >= 128. This is faster than using `&&` (branchy).
+            return til::at(filter, ch & 127) & (ch < 128);
+        });
+        return str;
+    }
+
     // std::string_view::starts_with support for C++17.
     template<typename T, typename Traits>
     constexpr bool starts_with(const std::basic_string_view<T, Traits>& str, const std::basic_string_view<T, Traits>& prefix) noexcept


Test patch: