#6031: DeclarationManager is now more tolerant when it comes to tryin…

…g to remove decls from files.
codereader · Aug 14, 2022 · 40baf89 · 40baf89
1 parent a1dbe37
commit 40baf89
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 27 deletions.
diff --git a/radiant/ui/particles/ParticleEditor.cpp b/radiant/ui/particles/ParticleEditor.cpp
@@ -1012,8 +1012,11 @@ void ParticleEditor::updateWidgetsFromParticle()
     updateWidgetsFromStage();
 
     // Update outfile label
+    std::string origName = getParticleNameFromIter(_selectedDefIter);
+    auto origDef = GlobalParticlesManager().getDefByName(origName);
+
     fs::path outFile = GlobalGameManager().getModPath();
-    outFile /= _currentDef->getBlockSyntax().fileInfo.fullPath();
+    outFile /= origDef->getBlockSyntax().fileInfo.fullPath();
 
 	findNamedObject<wxStaticText>(this, "ParticleEditorSaveNote")->SetLabelMarkup(
 		fmt::format(_("Note: changes will be written to the file <i>{0}</i>"), outFile.string()));
@@ -1279,7 +1282,8 @@ void ParticleEditor::setupEditParticle()
     std::string temporaryParticleName = selectedName + EDIT_SUFFIX;
 
     _currentDef = GlobalParticlesManager().findOrInsertParticleDef(temporaryParticleName);
-    _currentDef->setFilename(os::getFilename(def->getBlockSyntax().fileInfo.name));
+    // Set the edit particle to an empty file info, it will be filled on saving
+    _currentDef->setFileInfo(vfs::FileInfo());
 
     _currentDef->copyFrom(def);
 
@@ -1473,7 +1477,7 @@ IParticleDef::Ptr ParticleEditor::createAndSelectNewParticle()
     // Good filename, good destination file, we're set to go
     auto particle = GlobalParticlesManager().findOrInsertParticleDef(particleName);
 
-    particle->setFilename(destFile);
+    particle->setFilename("particles/" + destFile);
 
     // Re-load the particles list
     populateParticleDefList();

diff --git a/radiantcore/decl/DeclarationManager.cpp b/radiantcore/decl/DeclarationManager.cpp
@@ -309,7 +309,6 @@ void DeclarationManager::removeDeclaration(Type type, const std::string& name)
 
         if (decl != decls.end())
         {
-            // This will throw if the decl is stored in an archive
             removeDeclarationFromFile(decl->second);
 
             // Clear out this declaration's syntax block
@@ -328,7 +327,7 @@ void DeclarationManager::removeDeclaration(Type type, const std::string& name)
 namespace
 {
 
-void removeDeclarationFromSyntaxTree(const parser::DefSyntaxTree::Ptr& syntaxTree, const std::string& declName)
+bool removeDeclarationFromSyntaxTree(const parser::DefSyntaxTree::Ptr& syntaxTree, const std::string& declName)
 {
     // Remove the declaration from the tree
     std::vector<parser::DefSyntaxNode::Ptr> nodesToRemove;
@@ -384,16 +383,12 @@ void removeDeclarationFromSyntaxTree(const parser::DefSyntaxTree::Ptr& syntaxTre
         }
     }
 
-    if (nodesToRemove.empty())
-    {
-        rWarning() << "Could not locate the decl block " << declName << std::endl;
-        return;
-    }
-
     for (const auto& node : nodesToRemove)
     {
         syntaxTree->getRoot()->removeChildNode(node);
     }
+
+    return !nodesToRemove.empty(); // true if we removed one node or more
 }
 
 }
@@ -412,37 +407,43 @@ void DeclarationManager::removeDeclarationFromFile(const IDeclaration::Ptr& decl
 
     auto fullPath = GlobalFileSystem().findFile(syntax.fileInfo.fullPath());
 
-    if (fullPath.empty() || !fs::exists(fullPath)) throw std::logic_error("Physical file not found: " + syntax.fileInfo.fullPath());
+    if (fullPath.empty() || !fs::exists(fullPath))
+    {
+        return;
+    }
 
     fullPath += syntax.fileInfo.fullPath();
 
-    // Open a temporary file
-    stream::TemporaryOutputStream tempStream(fullPath);
+    // Load the syntax tree from the existing file
+    std::ifstream existingFile(fullPath);
 
-    auto& stream = tempStream.getStream();
-
-    std::ifstream inheritStream(fullPath);
-
-    if (!inheritStream.is_open())
+    if (!existingFile.is_open())
     {
         throw std::runtime_error(fmt::format(_("Cannot open file for reading: {0}"), fullPath));
     }
 
     // Parse the existing file into a syntax tree for manipulation
-    parser::DefBlockSyntaxParser<std::istream> parser(inheritStream);
+    parser::DefBlockSyntaxParser<std::istream> parser(existingFile);
 
     auto syntaxTree = parser.parse();
-    inheritStream.close();
+    existingFile.close();
 
-    // Move the old file to .bak before overwriting it
-    os::moveToBackupFile(fullPath);
+    // Try to remove the decl from the syntax tree
+    // Returns false if the decl could not be located (decl has not been saved to this file then)
+    if (removeDeclarationFromSyntaxTree(syntaxTree, decl->getDeclName()))
+    {
+        // Open a temporary file
+        stream::TemporaryOutputStream tempStream(fullPath);
+        auto& stream = tempStream.getStream();
 
-    removeDeclarationFromSyntaxTree(syntaxTree, decl->getDeclName());
+        // Move the old file to .bak before overwriting it
+        os::moveToBackupFile(fullPath);
 
-    // Export the modified syntax tree
-    stream << syntaxTree->getString();
+        // Export the modified syntax tree
+        stream << syntaxTree->getString();
 
-    tempStream.closeAndReplaceTargetFile();
+        tempStream.closeAndReplaceTargetFile();
+    }
 }
 
 bool DeclarationManager::renameDeclaration(Type type, const std::string& oldName, const std::string& newName)