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.

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

fix(text) escape value before rendering styles #20

Merged
merged 3 commits into from
Feb 27, 2023
Merged

fix(text) escape value before rendering styles #20

merged 3 commits into from
Feb 27, 2023

Conversation

tombell
Copy link
Contributor

@tombell tombell commented Feb 23, 2023

Update to essentially render lipgloss styles for values after escaping and indenting.

  • Add l *looger receiver to writeEscapedForOutput for access to l.noStyles
  • Add l *logger receiver to writeIdent to be able to call the updated writeEscapedForOutput
  • Update writeEscapedForOutput to call ValueStyle.Render after escaping the values, or if the values don't need need escaping

Fix #19

@codecov
Copy link

codecov bot commented Feb 23, 2023

Codecov Report

Merging #20 (84f189b) into main (17a651c) will increase coverage by 4.23%.
The diff coverage is 70.58%.

@@            Coverage Diff             @@
##             main      #20      +/-   ##
==========================================
+ Coverage   49.85%   54.09%   +4.23%     
==========================================
  Files          23       23              
  Lines         700      721      +21     
==========================================
+ Hits          349      390      +41     
+ Misses        335      317      -18     
+ Partials       16       14       -2     
Impacted Files Coverage Δ
text.go 53.63% <70.58%> (+7.70%) ⬆️
level.go 92.85% <0.00%> (+35.71%) ⬆️
styles.go 42.85% <0.00%> (+42.85%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@tombell tombell marked this pull request as ready for review February 23, 2023 19:07
@aymanbagabas
Copy link
Member

Thank you @tombell! Adding a test for this would be nice :)

@tombell
Copy link
Contributor Author

tombell commented Feb 23, 2023

@aymanbagabas just figuring out how the existing test broke first :)

@tombell
Copy link
Contributor Author

tombell commented Feb 23, 2023

@aymanbagabas trying to figure out the best approach to adding some test cases. Since noStyles will always be false not sure how to get it to render with the escape codes to check the output. Do you have any suggestions?

@aymanbagabas
Copy link
Member

@tombell you could modify the *logger struct noStyles value directly in the tests

func TestStyles(t *testing.T) {
  var buf bytes.Buffer
  l := New(WithOutput(&buf)).(*logger)
  l.noStyles = false
  ...
}

@tombell
Copy link
Contributor Author

tombell commented Feb 23, 2023

@aymanbagabas believe I've got this sorted now, all pushed up.

@tombell
Copy link
Contributor Author

tombell commented Feb 24, 2023

I believe I've refactored enough to simplify the solution.

@aymanbagabas aymanbagabas changed the base branch from main to fix-value-style February 27, 2023 17:33
@aymanbagabas aymanbagabas merged commit cddf0df into charmbracelet:fix-value-style Feb 27, 2023
aymanbagabas added a commit that referenced this pull request Feb 27, 2023

Verified

This commit was signed with the committer’s verified signature.
aymanbagabas Ayman Bagabas
Fixes: cddf0df ("fix(text) escape value before rendering styles (#20)")
@tombell tombell deleted the fix-value-styles-escaping branch February 27, 2023 18:25
aymanbagabas pushed a commit that referenced this pull request Feb 27, 2023
* fix(text) escape value before rendering style

* fix(text) render value style before escaping

* test(text) add additional test cases
aymanbagabas added a commit that referenced this pull request Feb 27, 2023
Fixes: cddf0df ("fix(text) escape value before rendering styles (#20)")
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.

Escaped escape codes when overriding ValueStyle and value needs quoting
2 participants