-
Notifications
You must be signed in to change notification settings - Fork 23
Fix objects being marked as dropped during script generation instead of execution #203
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
@shueybubbles 👋 This repository doesn't have Copilot instructions. With Copilot instructions, I can understand the repository better, work faster and produce higher quality PRs. I can generate a .github/copilot-instructions.md file for you automatically. Click here to open a pre-filled issue and assign it to me. I'll write the instructions, and then tag you for review. |
Co-authored-by: shueybubbles <2224906+shueybubbles@users.noreply.github.com>
…pting operations Co-authored-by: shueybubbles <2224906+shueybubbles@users.noreply.github.com>
|
@copilot |
You're absolutely right. After investigating further, I see that Scripter doesn't use Recording mode and my fix was likely incorrect. The real issue appears to be that |
Based on feedback, the original fix was incorrect. The real issue is that objects were being marked as dropped during script generation when they should only be marked as dropped during actual execution. The fix changes the condition from !ExecutionManager.Recording to sp.ForDirectExecution to properly distinguish between: - Script generation (ForDirectExecution=false): Don't mark objects as dropped - Actual execution (ForDirectExecution=true): Mark objects as dropped This is the correct way to handle the issue since Scripter doesn't use Recording mode. Co-authored-by: shueybubbles <2224906+shueybubbles@users.noreply.github.com>
This PR fixes an issue where calling scripting operations multiple times on the same SMO object would fail with the error:
Problem
The issue occurs when users call scripting operations like this:
Root Cause
The root cause was that
SqlSmoObject.MarkDropped()was being called during script generation when it should only be called during actual execution. The original code used!ExecutionManager.Recordingto distinguish between modes, butScripterdoesn't use Recording mode, causing objects to be incorrectly marked as dropped during script generation.Solution
The fix properly distinguishes between script generation and actual execution by using the
ForDirectExecutionflag fromScriptingPreferences:When
ForDirectExecution = true, we're performing an actual drop operation and should mark the object as dropped. WhenForDirectExecution = false, we're just generating a script and should not change the object's state.Testing
Added a comprehensive regression test
Scripter_MultipleScriptCallsWithDrops_ShouldNotFailWithObjectDroppedException()that reproduces the exact scenario from the issue and verifies multiple scripting calls work correctly.Fixes #202.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.