Skip to content

Conversation

@mejiaej
Copy link
Contributor

@mejiaej mejiaej commented Mar 27, 2021

My goal is to add the ability to insert the parameter anywhere in the query. I'm modifying the execution of query shortcuts by first checking if {arg} is present, if true just replace it with the parameter if not add the parameter at the end of the query just like it's been working until now.

This PR fixes #14893

@ghost
Copy link

ghost commented Mar 27, 2021

CLA assistant check
All CLA requirements met.

@coveralls
Copy link

coveralls commented Mar 27, 2021

Pull Request Test Coverage Report for Build 703278774

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 179 unchanged lines in 10 files lost coverage.
  • Overall coverage decreased (-0.007%) to 43.208%

Files with Coverage Reduction New Missed Lines %
extensions/sql-database-projects/src/models/project.ts 1 95.47%
src/sql/workbench/contrib/notebook/browser/models/fileNotebookInput.ts 1 44.44%
src/sql/workbench/contrib/notebook/browser/models/untitledNotebookInput.ts 1 75.0%
extensions/azurecore/src/azureResource/utils.ts 3 6.6%
src/sql/platform/serialization/common/serializationService.ts 11 57.89%
src/sql/workbench/contrib/query/browser/actions.ts 21 38.54%
extensions/azurecore/src/extension.ts 22 0%
extensions/sql-database-projects/src/controllers/projectController.ts 31 59.71%
extensions/sql-database-projects/src/dialogs/addDatabaseReferenceDialog.ts 36 60.72%
src/sql/workbench/contrib/query/browser/messagePanel.ts 52 2.91%
Totals Coverage Status
Change from base Build 691450289: -0.007%
Covered Lines: 25482
Relevant Lines: 53795

💛 - Coveralls

Copy link
Member

@kburtram kburtram left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems reasonable to me. @alanrenmsft what do you think?

@kburtram kburtram requested a review from alanrenmsft March 27, 2021 23:51
@kburtram
Copy link
Member

@mejiaej could you please make the update @Charles-Gagnon suggests regarding doing a global replace? Thanks!

@mejiaej
Copy link
Contributor Author

mejiaej commented Mar 30, 2021

@kburtram can do, I'll do it later today. I'm on working hours right now 😂

@kburtram
Copy link
Member

@mejiaej thanks! no rush. It would be good to have that update before we can merge.

@Charles-Gagnon Charles-Gagnon merged commit 00d2fad into microsoft:main Mar 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add selected text as parameter in query shortcuts

5 participants