Skip to content
This repository has been archived by the owner on Sep 27, 2024. It is now read-only.

Remove trailingTrimmed from withNBSP #884

Merged
merged 1 commit into from
Nov 21, 2023

Conversation

langleyd
Copy link
Contributor

@langleyd langleyd commented Nov 21, 2023

Fixes an issue with double space -> . conversion breaking formatting:

RPReplay_Final1700486454.MP4

The trimming in withNBSP was added back when we were using zero-width spaces(ZWSP) as a part of the initial paragraphs work.
We no longer use ZWSP.

In #831 @aringenbach changed it to just remove trailing(not leading) space to fix that issue. Maybe it's not needed at all anymore.

Nit: Either way it probably shouldn't be a function called withNBSP as it's not clear form the call site that it is also trimming.

So unless somebody knows of a problem it is currently solving, lets remove it to fix this issue. If it causes a regression we can then solve for both cases.

@Velin92 Do you happen to remember why the trimming was needed here? And if maybe it's not needed anymore now that we use just paragraphs and no ZWSP?

Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (320f5c1) 87.44% compared to head (397b98c) 89.39%.
Report is 2 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main     #884      +/-   ##
============================================
+ Coverage     87.44%   89.39%   +1.95%     
============================================
  Files           165       86      -79     
  Lines         18964    15324    -3640     
  Branches       1048        0    -1048     
============================================
- Hits          16583    13699    -2884     
+ Misses         2086     1625     -461     
+ Partials        295        0     -295     
Flag Coverage Δ
uitests ?
uitests-android ?
uitests-ios ?
unittests 89.39% <ø> (+3.86%) ⬆️
unittests-android ?
unittests-ios ?
unittests-rust 89.39% <ø> (ø)

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@nimau nimau self-requested a review November 21, 2023 11:38
@nimau nimau removed their assignment Nov 21, 2023
Copy link
Contributor

@nimau nimau left a comment

Choose a reason for hiding this comment

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

LGTM

@langleyd langleyd added the iOS label Nov 21, 2023
@langleyd langleyd merged commit 675502e into main Nov 21, 2023
9 checks passed
@langleyd langleyd deleted the langleyd/fix_double_space_index_issue branch November 21, 2023 13:41
@jonnyandrew jonnyandrew mentioned this pull request Nov 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants