Return strings directly from read_file_as_utf8_string_if_exists (#17667)

* Every single place that called `read_file_as_utf8_string_if_exists`
  would immediately do a `.value_or(std::string{})`.
  As such, the function now returns a string directly.
* There was just one caller to `read_file_as_utf8_string`
  and it only cared about files that are non-empty.
  As such, the specialization got removed.

Both of these make sense to me, as in practice there's seldom
a difference between an empty file and a non-existent one.

## Validation Steps Performed
* Compiles ✅
* Starts ✅
* Deleting the `settings.json` contents triggers a reload ✅

src/cascadia/TerminalSettingsModel/ActionMap.cpp

@@ -923,13 +923,12 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
     {
         // This returns an empty string if we fail to load the file.
         std::filesystem::path localSnippetsPath{ std::wstring_view{ currentWorkingDirectory + L"\\.wt.json" } };
-        const auto localTasksFileContents = til::io::read_file_as_utf8_string_if_exists(localSnippetsPath);
+        const auto data = til::io::read_file_as_utf8_string_if_exists(localSnippetsPath);
-        if (!localTasksFileContents.has_value() || localTasksFileContents->empty())
+        if (data.empty())
         {
             return {};
         }
 
-        const auto& data = *localTasksFileContents;
         Json::Value root;
         std::string errs;
         const std::unique_ptr<Json::CharReader> reader{ Json::CharReaderBuilder{}.newCharReader() };

src/cascadia/TerminalSettingsModel/ApplicationState.cpp

@@ -146,7 +146,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
         std::unique_ptr<Json::CharReader> reader{ Json::CharReaderBuilder{}.newCharReader() };
 
         // First get shared state out of `state.json`.
-        const auto sharedData = _readSharedContents().value_or(std::string{});
+        const auto sharedData = _readSharedContents();
         if (!sharedData.empty())
         {
             Json::Value root;
@@ -165,7 +165,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
                 FromJson(root, FileSource::Shared);
 
                 // Then, try and get anything in elevated-state
-                if (const auto localData{ _readLocalContents().value_or(std::string{}) }; !localData.empty())
+                if (const auto localData{ _readLocalContents() }; !localData.empty())
                 {
                     Json::Value root;
                     if (!reader->parse(localData.data(), localData.data() + localData.size(), &root, &errs))
@@ -216,7 +216,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
             // First load the contents of state.json into a json blob. This will
             // contain the Shared properties and the unelevated instance's Local
             // properties.
-            const auto sharedData = _readSharedContents().value_or(std::string{});
+            const auto sharedData = _readSharedContents();
             if (!sharedData.empty())
             {
                 if (!reader->parse(sharedData.data(), sharedData.data() + sharedData.size(), &root, &errs))
@@ -334,7 +334,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
     // - Read the contents of our "shared" state - state that should be shared
     //   for elevated and unelevated instances. This is things like the list of
     //   generated profiles, the command palette commandlines.
-    std::optional<std::string> ApplicationState::_readSharedContents() const
+    std::string ApplicationState::_readSharedContents() const
     {
         return til::io::read_file_as_utf8_string_if_exists(_sharedPath);
     }
@@ -346,7 +346,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
     //   those don't matter when unelevated).
     // - When elevated, this will DELETE `elevated-state.json` if it has bad
     //   permissions, so we don't potentially read malicious data.
-    std::optional<std::string> ApplicationState::_readLocalContents() const
+    std::string ApplicationState::_readLocalContents() const
     {
         return ::Microsoft::Console::Utils::IsRunningElevated() ?
                    til::io::read_file_as_utf8_string_if_exists(_elevatedPath, true) :

src/cascadia/TerminalSettingsModel/ApplicationState.h

@@ -95,9 +95,9 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
 
         Json::Value _toJsonWithBlob(Json::Value& root, FileSource parseSource) const noexcept;
 
-        std::optional<std::string> _readSharedContents() const;
+        std::string _readSharedContents() const;
         void _writeSharedContents(const std::string_view content) const;
-        std::optional<std::string> _readLocalContents() const;
+        std::string _readLocalContents() const;
         void _writeLocalContents(const std::string_view content) const;
     };
 }

src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp

@@ -237,9 +237,12 @@ void SettingsLoader::FindFragmentsAndMergeIntoUserSettings()
             {
                 try
                 {
-                    const auto content = til::io::read_file_as_utf8_string(fragmentExt.path());
+                    const auto content = til::io::read_file_as_utf8_string_if_exists(fragmentExt.path());
+                    if (!content.empty())
+                    {
                         _parseFragment(source, content, fragmentSettings);
                     }
+                }
                 CATCH_LOG();
             }
         }
@@ -934,7 +937,7 @@ Model::CascadiaSettings CascadiaSettings::LoadAll()
 try
 {
     FILETIME lastWriteTime{};
-    auto settingsString = til::io::read_file_as_utf8_string_if_exists(_settingsPath(), false, &lastWriteTime).value_or(std::string{});
+    auto settingsString = til::io::read_file_as_utf8_string_if_exists(_settingsPath(), false, &lastWriteTime);
     auto firstTimeSetup = settingsString.empty();
 
     // If it's the firstTimeSetup and a preview build, then try to
@@ -947,7 +950,7 @@ try
         {
             try
             {
-                settingsString = til::io::read_file_as_utf8_string_if_exists(_releaseSettingsPath()).value_or(std::string{});
+                settingsString = til::io::read_file_as_utf8_string_if_exists(_releaseSettingsPath());
                 releaseSettingExists = settingsString.empty() ? false : true;
             }
             catch (...)

src/inc/til/io.h

@@ -53,8 +53,8 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned"
         } // details
 
         // Tries to read a file somewhat atomically without locking it.
-        // Strips the UTF8 BOM if it exists.
+        // Returns an empty string if the file couldn't be opened.
-        _TIL_INLINEPREFIX std::string read_file_as_utf8_string(const std::filesystem::path& path, const bool elevatedOnly = false, FILETIME* lastWriteTime = nullptr)
+        _TIL_INLINEPREFIX std::string read_file_as_utf8_string_if_exists(const std::filesystem::path& path, const bool elevatedOnly = false, FILETIME* lastWriteTime = nullptr)
         {
             // From some casual observations we can determine that:
             // * ReadFile() always returns the requested amount of data (unless the file is smaller)
@@ -69,7 +69,16 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned"
                                                     OPEN_EXISTING,
                                                     FILE_ATTRIBUTE_NORMAL,
                                                     nullptr) };
-                THROW_LAST_ERROR_IF(!file);
+
+                if (!file)
+                {
+                    const auto gle = GetLastError();
+                    if (gle == ERROR_FILE_NOT_FOUND)
+                    {
+                        return {};
+                    }
+                    THROW_WIN32(gle);
+                }
 
                 // Open the file _first_, then check if it has the right
                 // permissions. This prevents a "Time-of-check to time-of-use"
@@ -92,7 +101,7 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned"
                         LOG_LAST_ERROR_IF(!DeleteFile(path.c_str()));
 
                         // Exit early, because obviously there's nothing to read from the deleted file.
-                        return "";
+                        return {};
                     }
                 }
 
@@ -137,24 +146,6 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned"
             THROW_WIN32_MSG(ERROR_READ_FAULT, "file size changed while reading");
         }
 
-        // Same as read_file_as_utf8_string, but returns an empty optional, if the file couldn't be opened.
-        _TIL_INLINEPREFIX std::optional<std::string> read_file_as_utf8_string_if_exists(const std::filesystem::path& path, const bool elevatedOnly = false, FILETIME* lastWriteTime = nullptr)
-        {
-            try
-            {
-                return { read_file_as_utf8_string(path, elevatedOnly, lastWriteTime) };
-            }
-            catch (const wil::ResultException& exception)
-            {
-                if (exception.GetErrorCode() == HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND))
-                {
-                    return {};
-                }
-
-                throw;
-            }
-        }
-
         _TIL_INLINEPREFIX void write_utf8_string_to_file(const std::filesystem::path& path, const std::string_view& content, const bool elevatedOnly = false, FILETIME* lastWriteTime = nullptr)
         {
             SECURITY_ATTRIBUTES sa;