Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Paste as BicepParams #15897

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Paste as BicepParams #15897

wants to merge 7 commits into from

Conversation

miqm
Copy link
Collaborator

@miqm miqm commented Dec 16, 2024 •
edited
Loading

Fixes #15769

Changes

  • Pasting json values in bicepparam is converted to bicep native object
  • Pasting entire json parameters file is being decompiled into param statements
    • if using directive is already there - it's not being added for the second time
    • overlapping param will be duplicated though

Contributing a feature

  • I have opened a new issue for the proposal, or commented on an existing one, and ensured that the Bicep maintainers are good with the design of the feature being implemented
  • I have included "Fixes #{issue_number}" in the PR description, so GitHub can link to the issue and close it when the PR is merged
  • I have appropriate test coverage of my new feature

@codecov-commenter
Copy link

codecov-commenter commented Dec 16, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 9.37%. Comparing base (108d816) to head (d0e9f35).
Report is 1720 commits behind head on main.

❗ There is a different number of reports uploaded between BASE (108d816) and HEAD (d0e9f35). Click for more details.

HEAD has 4 uploads less than BASE
Flag BASE (108d816) HEAD (d0e9f35)
dotnet 4 0
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##             main   #15897       +/-   ##
===========================================
- Coverage   94.28%    9.37%   -84.91%     
===========================================
  Files        1113        7     -1106     
  Lines      100791      288   -100503     
  Branches     8734      123     -8611     
===========================================
- Hits        95028       27    -95001     
+ Misses       4595      261     -4334     
+ Partials     1168        0     -1168
Flag Coverage Δ
dotnet ?
typescript 9.37% <ø> (+1.66%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 1104 files with indirect coverage changes

miqm
miqm commented Dec 16, 2024
View reviewed changes
src/vscode-bicep/src/commands/pasteAsBicep.ts
@@ -144,6 +146,7 @@ export class PasteAsBicepCommand implements Command {
rangeLength,
jsonContent,
queryCanPaste,
languageId
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

should we add languageId to telemetry?

Copy link
Contributor

Choose a reason for hiding this comment

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

sounds good

@StephenWeatherford
Copy link
Contributor

@miqm Been OOF, I can take a look tomorrow.

StephenWeatherford
StephenWeatherford reviewed Dec 19, 2024
View reviewed changes
src/Bicep.Decompiler/BicepDecompiler.cs
}

private ProgramSyntax DecompileParametersFile(string jsonInput, Uri entryBicepparamUri, Uri? bicepFileUri)
private ProgramSyntax DecompileParametersFile(string jsonInput, Uri entryBicepparamUri, Uri? bicepFileUri, DecompileParamOptions options)
Copy link
Contributor

Choose a reason for hiding this comment

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

TODO

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, comment for me... :-)

@StephenWeatherford
Copy link
Contributor

I've left some commets, not done yet.

@miqm
Copy link
Collaborator Author

miqm commented Dec 30, 2024 •
edited
Loading

@StephenWeatherford I've fixed highlighted problems

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shenglol shenglol Awaiting requested review from shenglol

@anthony-c-martin anthony-c-martin Awaiting requested review from anthony-c-martin

@StephenWeatherford StephenWeatherford Awaiting requested review from StephenWeatherford

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Paste as bicep in bicepparam files doesn't work
3 participants
@miqm @codecov-commenter @StephenWeatherford