Feedback update, rework string to bool

dkbennett · dkbennett · commit 9221c20363d3 · 2025-12-15T10:58:03.000-08:00
diff --git a/src/AppInstallerCLICore/Workflows/SourceFlow.cpp b/src/AppInstallerCLICore/Workflows/SourceFlow.cpp
@@ -289,7 +289,7 @@ namespace AppInstaller::CLI::Workflow
                 isExplicit = Utility::TryConvertStringToBool(context.Args.GetArg(Execution::Args::Type::SourceEditExplicit));
             }
 
-            Repository::SourceEdit edits{ std::optional<bool>{ isExplicit } };
+            Repository::SourceEdit edits{ isExplicit };
             if (!targetSource.RequiresChanges(edits))
             {
                 context.Reporter.Info() << Resource::String::SourceEditNoChanges(Utility::LocIndView{ sd.Name }) << std::endl;
@@ -301,7 +301,7 @@ namespace AppInstaller::CLI::Workflow
 
             // Output updated source information. Since only Explicit is editable, we will only list that field. The name of the source being edited is listed prior to the edits.
             Execution::TableOutput<3> table(context.Reporter, { Resource::String::SourceListField, Resource::String::SourceEditOldValue, Resource::String::SourceEditNewValue });
-            table.OutputLine({ Resource::LocString(Resource::String::SourceListExplicit), std::string{ Utility::ConvertBoolToString(oldExplicitValue) }, std::string{ Utility::ConvertBoolToString(isExplicit) } });
+            table.OutputLine({ Resource::LocString(Resource::String::SourceListExplicit), std::string{ Utility::ConvertBoolToString(oldExplicitValue) }, std::string{ Utility::ConvertBoolToString(isExplicit.value()) } });
             table.Complete();
         }
     }
diff --git a/src/AppInstallerCLITests/TestSource.cpp b/src/AppInstallerCLITests/TestSource.cpp
@@ -454,15 +454,6 @@ namespace TestCommon
         return true;
     }
 
-    bool TestSourceFactory::Edit(SourceDetails& details, const SourceEdit& edits)
-    {
-        if (OnEdit)
-        {
-            OnEdit(details, edits);
-        }
-        return true;
-    }
-
     // Make copies of self when requested.
     TestSourceFactory::operator std::function<std::unique_ptr<ISourceFactory>()>()
     {
@@ -475,12 +466,6 @@ namespace TestCommon
         return source.Add(progress);
     }
 
-    bool EditSource(const AppInstaller::Repository::SourceDetails& details, const AppInstaller::Repository::SourceEdit& edits)
-    {
-        Repository::Source source{ details.Name, details.Explicit };
-        return source.Edit(edits);
-    }
-
     bool UpdateSource(std::string_view name, AppInstaller::IProgressCallback& progress)
     {
         Repository::Source source{ name };
diff --git a/src/AppInstallerCLITests/TestSource.h b/src/AppInstallerCLITests/TestSource.h
@@ -187,7 +187,6 @@ namespace TestCommon
         using AddFunctor = std::function<void(AppInstaller::Repository::SourceDetails&)>;
         using UpdateFunctor = std::function<void(const AppInstaller::Repository::SourceDetails&)>;
         using RemoveFunctor = std::function<void(const AppInstaller::Repository::SourceDetails&)>;
-        using EditFunctor = std::function<void(AppInstaller::Repository::SourceDetails&, const AppInstaller::Repository::SourceEdit& edits)>;
 
         TestSourceFactory(OpenFunctor open) : OnOpen(std::move(open)) {}
         TestSourceFactory(OpenFunctorWithCustomHeader open) : OnOpenWithCustomHeader(std::move(open)) {}
@@ -198,7 +197,6 @@ namespace TestCommon
         bool Add(AppInstaller::Repository::SourceDetails& details, AppInstaller::IProgressCallback&) override;
         bool Update(const AppInstaller::Repository::SourceDetails& details, AppInstaller::IProgressCallback&) override;
         bool Remove(const AppInstaller::Repository::SourceDetails& details, AppInstaller::IProgressCallback&) override;
-        bool Edit(AppInstaller::Repository::SourceDetails& details, const AppInstaller::Repository::SourceEdit& edits) override;
 
         // Make copies of self when requested.
         operator std::function<std::unique_ptr<AppInstaller::Repository::ISourceFactory>()>();
@@ -209,11 +207,9 @@ namespace TestCommon
         AddFunctor OnAdd;
         UpdateFunctor OnUpdate;
         RemoveFunctor OnRemove;
-        EditFunctor OnEdit;
     };
 
     bool AddSource(const AppInstaller::Repository::SourceDetails& details, AppInstaller::IProgressCallback& progress);
-    bool EditSource(AppInstaller::Repository::SourceDetails& details, const AppInstaller::Repository::SourceEdit& edits);
     bool UpdateSource(std::string_view name, AppInstaller::IProgressCallback& progress);
     bool RemoveSource(std::string_view name, AppInstaller::IProgressCallback& progress);
     AppInstaller::Repository::Source OpenSource(std::string_view name, AppInstaller::IProgressCallback& progress);
diff --git a/src/AppInstallerRepositoryCore/Microsoft/ConfigurableTestSourceFactory.cpp b/src/AppInstallerRepositoryCore/Microsoft/ConfigurableTestSourceFactory.cpp
@@ -154,11 +154,6 @@ namespace AppInstaller::Repository::Microsoft
             {
                 return true;
             }
-
-            bool Edit(SourceDetails&, const SourceEdit&) override final
-            {
-                return true;
-            }
         };
     }
 
diff --git a/src/AppInstallerRepositoryCore/Microsoft/PreIndexedPackageSourceFactory.cpp b/src/AppInstallerRepositoryCore/Microsoft/PreIndexedPackageSourceFactory.cpp
@@ -254,17 +254,6 @@ namespace AppInstaller::Repository::Microsoft
                 return UpdateBase(details, true, progress);
             }
 
-            // Applies the edits to the source.
-            bool Edit(SourceDetails& details, const SourceEdit& edits) override final
-            {
-                if (edits.Explicit.has_value())
-                {
-                    details.Explicit = edits.Explicit.value();
-                }
-
-                return true;
-            }
-
             // Retrieves the currently cached version of the package.
             virtual std::optional<Msix::PackageVersion> GetCurrentVersion(const SourceDetails& details) = 0;
 
diff --git a/src/AppInstallerRepositoryCore/Microsoft/PredefinedInstalledSourceFactory.cpp b/src/AppInstallerRepositoryCore/Microsoft/PredefinedInstalledSourceFactory.cpp
@@ -445,11 +445,6 @@ namespace AppInstaller::Repository::Microsoft
                 // Similar to add, remove should never be needed.
                 THROW_HR(E_NOTIMPL);
             }
-
-            bool Edit(SourceDetails&, const SourceEdit&) override final
-            {
-                THROW_HR(E_NOTIMPL);
-            }
         };
     }
 
diff --git a/src/AppInstallerRepositoryCore/Microsoft/PredefinedWriteableSourceFactory.cpp b/src/AppInstallerRepositoryCore/Microsoft/PredefinedWriteableSourceFactory.cpp
@@ -44,11 +44,6 @@ namespace AppInstaller::Repository::Microsoft
                 // Similar to add, remove should never be needed.
                 THROW_HR(E_NOTIMPL);
             }
-
-            bool Edit(SourceDetails&, const SourceEdit&) override final
-            {
-                THROW_HR(E_NOTIMPL);
-            }
         };
 
         struct PredefinedWriteableSourceReference : public ISourceReference
diff --git a/src/AppInstallerRepositoryCore/PackageTrackingCatalog.cpp b/src/AppInstallerRepositoryCore/PackageTrackingCatalog.cpp
@@ -125,11 +125,6 @@ namespace AppInstaller::Repository
                 THROW_HR(E_NOTIMPL);
             }
 
-            bool Edit(SourceDetails&, const SourceEdit&) override final
-            {
-                THROW_HR(E_NOTIMPL);
-            }
-
             bool Remove(const SourceDetails& details, IProgressCallback& progress) override final
             {
                 THROW_HR_IF(E_INVALIDARG, !Utility::CaseInsensitiveEquals(details.Type, PackageTrackingCatalogSourceFactory::Type()));
diff --git a/src/AppInstallerRepositoryCore/Public/winget/RepositorySource.h b/src/AppInstallerRepositoryCore/Public/winget/RepositorySource.h
@@ -233,9 +233,6 @@ namespace AppInstaller::Repository
         // Constructor for a source to be added.
         Source(std::string_view name, std::string_view arg, std::string_view type, SourceTrustLevel trustLevel, bool isExplicit);
 
-        // Constructor for a source to be edited.
-        Source(std::string_view name, std::optional<bool> isExplicit);
-
         // Constructor for creating a composite source from a list of available sources.
         Source(const std::vector<Source>& availableSources);
 
@@ -343,7 +340,7 @@ namespace AppInstaller::Repository
         bool Remove(IProgressCallback& progress);
 
         // Edit source. Source edit command.
-        bool Edit(const SourceEdit& edits);
+        void Edit(const SourceEdit& edits);
 
         // Determines if this source is a valid edit of otherSource.
         // Returns true if this source qualifies as an edit of the other source.
diff --git a/src/AppInstallerRepositoryCore/RepositorySource.cpp b/src/AppInstallerRepositoryCore/RepositorySource.cpp
@@ -135,12 +135,6 @@ namespace AppInstaller::Repository
             return AddOrUpdateFromDetails(details, &ISourceFactory::Update, progress);
         }
 
-        bool EditSource(SourceDetails& details, const SourceEdit& edits)
-        {
-            auto factory = ISourceFactory::GetForType(details.Type);
-            return factory->Edit(details, edits);
-        }
-
         AddOrUpdateResult BackgroundUpdateSourceFromDetails(SourceDetails& details, IProgressCallback& progress)
         {
             return AddOrUpdateFromDetails(details, &ISourceFactory::BackgroundUpdate, progress);
@@ -493,29 +487,6 @@ namespace AppInstaller::Repository
         m_sourceReferences.emplace_back(CreateSourceFromDetails(details));
     }
 
-    Source::Source(std::string_view name, std::optional<bool> isExplicit)
-    {
-        SourceList sourceList;
-        auto source = sourceList.GetCurrentSource(name);
-        if (!source)
-        {
-            AICLI_LOG(Repo, Info, << "Named source requested, but not found: " << name);
-        }
-        else
-        {
-            AICLI_LOG(Repo, Info, << "Named source requested, found: " << source->Name);
-
-            // This is intended to support Editing a source, and at present only the Explicit
-            // property of SourceDetails can be edited.
-            if (isExplicit.has_value())
-            {
-                source->Explicit = isExplicit.value();
-            }
-
-            m_sourceReferences.emplace_back(CreateSourceFromDetails(*source));
-        }
-    }
-
     Source::Source(const std::vector<Source>& availableSources)
     {
         std::shared_ptr<CompositeSource> compositeSource = std::make_shared<CompositeSource>("*CompositeSource");
@@ -1020,7 +991,7 @@ namespace AppInstaller::Repository
         return result;
     }
 
-    bool Source::Edit(const SourceEdit& edits)
+    void Source::Edit(const SourceEdit& edits)
     {
         THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_INVALID_STATE), m_isSourceToBeAdded || m_sourceReferences.size() != 1 || m_source);
 
@@ -1030,15 +1001,17 @@ namespace AppInstaller::Repository
         // This is intentionally the same policy checks as Remove. If the source cannot be removed then it cannot be edited.
         EnsureSourceIsRemovable(details);
 
-        // Apply the edits and update source list.
-        bool result = EditSource(details, edits);
-        if (result)
+        if (RequiresChanges(edits))
         {
+            if (edits.Explicit.has_value())
+            {
+                details.Explicit = edits.Explicit.value();
+            }
+
+            // Apply the edits and update source list.
             SourceList sourceList;
             sourceList.EditSource(details);
         }
-
-        return result;
     }
 
     bool Source::RequiresChanges(const SourceEdit& edits)
diff --git a/src/AppInstallerRepositoryCore/Rest/RestSourceFactory.cpp b/src/AppInstallerRepositoryCore/Rest/RestSourceFactory.cpp
@@ -134,12 +134,6 @@ namespace AppInstaller::Repository::Rest
                 THROW_HR_IF(E_INVALIDARG, !Utility::CaseInsensitiveEquals(details.Type, RestSourceFactory::Type()));
                 return true;
             }
-
-            bool Edit(SourceDetails& details, const SourceEdit&) override final
-            {
-                THROW_HR_IF(E_INVALIDARG, !Utility::CaseInsensitiveEquals(details.Type, RestSourceFactory::Type()));
-                return true;
-            }
         };
     }
 
diff --git a/src/AppInstallerRepositoryCore/SourceFactory.h b/src/AppInstallerRepositoryCore/SourceFactory.h
@@ -41,9 +41,6 @@ namespace AppInstaller::Repository
         // Return value indicates whether the action completed.
         virtual bool Remove(const SourceDetails& details, IProgressCallback& progress) = 0;
 
-        // Edit the given source with the provided edits.
-        virtual bool Edit(SourceDetails& details, const SourceEdit& edits) = 0;
-
         // Gets the factory for the given type.
         static std::unique_ptr<ISourceFactory> GetForType(std::string_view type);
     };
diff --git a/src/AppInstallerSharedLib/AppInstallerStrings.cpp b/src/AppInstallerSharedLib/AppInstallerStrings.cpp
@@ -943,11 +943,19 @@ namespace AppInstaller::Utility
     {
         try
         {
-            bool value;
-            std::istringstream(std::string(input)) >> std::boolalpha >> value;
-            return { value };
+            if (ICUCaseInsensitiveEquals(input, "false"sv))
+            {
+                return { false };
+            }
+
+            if (ICUCaseInsensitiveEquals(input, "true"sv))
+            {
+                return { true };
+            }
+
+            return {};
         }
-        catch
+        catch (...)
         {
             return {};
         }
diff --git a/src/Microsoft.Management.Deployment/PackageManager.cpp b/src/Microsoft.Management.Deployment/PackageManager.cpp
@@ -1467,13 +1467,7 @@ namespace winrt::Microsoft::Management::Deployment::implementation
             THROW_HR_IF(APPINSTALLER_CLI_ERROR_SOURCE_NAME_DOES_NOT_EXIST, !matchingSource.has_value());
             ::AppInstaller::Repository::Source sourceToEdit = ::AppInstaller::Repository::Source{ matchingSource.value().Name };
 
-            auto isExplicit = matchingSource.value().Explicit;
-            if (options.Explicit() != OptionalBoolean::Unspecified)
-            {
-                isExplicit = options.Explicit() == OptionalBoolean::True;
-            }
-
-            ::AppInstaller::Repository::SourceEdit edits{ std::optional<bool>{ isExplicit } };
+            ::AppInstaller::Repository::SourceEdit edits{ GetOptionalBoolean(options.Explicit())};
             if (sourceToEdit.RequiresChanges(edits))
             {
                 sourceToEdit.Edit(edits);

Original file line number	Diff line number	Diff line change
`@@ -289,7 +289,7 @@ namespace AppInstaller::CLI::Workflow`
`289`	`289`	`isExplicit = Utility::TryConvertStringToBool(context.Args.GetArg(Execution::Args::Type::SourceEditExplicit));`
`290`	`290`	`}`
`291`	`291`
`292`		`- Repository::SourceEdit edits{ std::optional<bool>{ isExplicit } };`
	`292`	`+ Repository::SourceEdit edits{ isExplicit };`
`293`	`293`	`if (!targetSource.RequiresChanges(edits))`
`294`	`294`	`{`
`295`	`295`	`context.Reporter.Info() << Resource::String::SourceEditNoChanges(Utility::LocIndView{ sd.Name }) << std::endl;`
`@@ -301,7 +301,7 @@ namespace AppInstaller::CLI::Workflow`
`301`	`301`
`302`	`302`	`// Output updated source information. Since only Explicit is editable, we will only list that field. The name of the source being edited is listed prior to the edits.`
`303`	`303`	`Execution::TableOutput<3> table(context.Reporter, { Resource::String::SourceListField, Resource::String::SourceEditOldValue, Resource::String::SourceEditNewValue });`
`304`		`- table.OutputLine({ Resource::LocString(Resource::String::SourceListExplicit), std::string{ Utility::ConvertBoolToString(oldExplicitValue) }, std::string{ Utility::ConvertBoolToString(isExplicit) } });`
	`304`	`+ table.OutputLine({ Resource::LocString(Resource::String::SourceListExplicit), std::string{ Utility::ConvertBoolToString(oldExplicitValue) }, std::string{ Utility::ConvertBoolToString(isExplicit.value()) } });`
`305`	`305`	`table.Complete();`
`306`	`306`	`}`
`307`	`307`	`}`
Original file line number	Diff line number	Diff line change
`@@ -154,11 +154,6 @@ namespace AppInstaller::Repository::Microsoft`
`154`	`154`	`{`
`155`	`155`	`return true;`
`156`	`156`	`}`
`157`		`-`
`158`		`- bool Edit(SourceDetails&, const SourceEdit&) override final`
`159`		`- {`
`160`		`- return true;`
`161`		`- }`
`162`	`157`	`};`
`163`	`158`	`}`
`164`	`159`
Original file line number	Diff line number	Diff line change
`@@ -445,11 +445,6 @@ namespace AppInstaller::Repository::Microsoft`
`445`	`445`	`// Similar to add, remove should never be needed.`
`446`	`446`	`THROW_HR(E_NOTIMPL);`
`447`	`447`	`}`
`448`		`-`
`449`		`- bool Edit(SourceDetails&, const SourceEdit&) override final`
`450`		`- {`
`451`		`- THROW_HR(E_NOTIMPL);`
`452`		`- }`
`453`	`448`	`};`
`454`	`449`	`}`
`455`	`450`
Original file line number	Diff line number	Diff line change
`@@ -125,11 +125,6 @@ namespace AppInstaller::Repository`
`125`	`125`	`THROW_HR(E_NOTIMPL);`
`126`	`126`	`}`
`127`	`127`
`128`		`- bool Edit(SourceDetails&, const SourceEdit&) override final`
`129`		`- {`
`130`		`- THROW_HR(E_NOTIMPL);`
`131`		`- }`
`132`		`-`
`133`	`128`	`bool Remove(const SourceDetails& details, IProgressCallback& progress) override final`
`134`	`129`	`{`
`135`	`130`	`THROW_HR_IF(E_INVALIDARG, !Utility::CaseInsensitiveEquals(details.Type, PackageTrackingCatalogSourceFactory::Type()));`