Skip to content

Fix format function {{, }} are escapes#752

Merged
mergify[bot] merged 2 commits intonektos:masterfrom
ChristopherHX:fix/format-escapes
Aug 9, 2021
Merged

Fix format function {{, }} are escapes#752
mergify[bot] merged 2 commits intonektos:masterfrom
ChristopherHX:fix/format-escapes

Conversation

@ChristopherHX
Copy link
Copy Markdown
Contributor

@ChristopherHX ChristopherHX commented Jul 16, 2021

Github transforms multiple expressions into one format expression while parsing and every non expression { is replaced by {{ and } by }}.

I assume vmformat should mimic githubs behavior, issues appeard during testing of https://github.com/ChristopherHX/github-actions-act-runner.

Haven't searched any documentation about this yet.

Some corner cases fixed too
format('{0}', github.undefined_value) should be an empty string not undefined.
format('{0}', '{1}', 'Hello') should be {1} not Hello.

How do I test the error handler?
vmFormat has no error output, so the tests cannot test for error messages or did I miss something?

@ChristopherHX ChristopherHX requested a review from a team as a code owner July 16, 2021 18:22
@codecov
Copy link
Copy Markdown

codecov Bot commented Jul 16, 2021

Codecov Report

Merging #752 (cd7ed35) into master (0f04942) will increase coverage by 1.52%.
The diff coverage is 58.02%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #752      +/-   ##
==========================================
+ Coverage   49.27%   50.79%   +1.52%     
==========================================
  Files          23       23              
  Lines        2401     2644     +243     
==========================================
+ Hits         1183     1343     +160     
- Misses       1090     1158      +68     
- Partials      128      143      +15     
Impacted Files Coverage Δ
pkg/container/docker_build.go 0.00% <0.00%> (ø)
pkg/container/docker_run.go 1.83% <0.00%> (-0.10%) ⬇️
pkg/model/workflow.go 25.29% <2.43%> (-0.43%) ⬇️
pkg/common/git.go 52.85% <30.15%> (-6.94%) ⬇️
pkg/runner/runner.go 68.96% <33.33%> (-7.51%) ⬇️
pkg/model/planner.go 34.56% <41.37%> (+1.48%) ⬆️
pkg/container/docker_pull.go 36.17% <64.70%> (+17.98%) ⬆️
pkg/runner/expression.go 85.38% <71.87%> (-1.26%) ⬇️
pkg/runner/step_context.go 75.00% <76.34%> (+6.05%) ⬆️
pkg/runner/run_context.go 80.36% <98.55%> (+3.95%) ⬆️
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0ff204b...cd7ed35. Read the comment docs.

@ChristopherHX ChristopherHX marked this pull request as draft July 16, 2021 19:58
@mergify

This comment has been minimized.

@mergify mergify Bot added the needs-work Extra attention is needed label Jul 16, 2021
@mergify mergify Bot removed the needs-work Extra attention is needed label Aug 8, 2021
@ChristopherHX ChristopherHX marked this pull request as ready for review August 8, 2021 10:00
@catthehacker
Copy link
Copy Markdown
Member

vmFormat has no error output, so the tests cannot test for error messages [...]?

correct

@mergify mergify Bot requested a review from a team August 9, 2021 08:41
@mergify mergify Bot merged commit 43d46aa into nektos:master Aug 9, 2021
@ChristopherHX ChristopherHX deleted the fix/format-escapes branch August 9, 2021 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants