highlight
highlight copied to clipboard
Addition/Deletion Injection Fixes
Hi @brendt, this is a continuation of fixes presented in #104 where addition/deletion is not able to get the line endings correctly in my use case scenario, similar to #115 on the gutters it cannot read the current line thus applying it always on the first line.
What I did was standardize the line endings to EOL so it can be more flexible when handling line endings.
$content = preg_replace("/(\r\n|\n|\r)/", PHP_EOL, $content);
While I was debugging this I've notice that the InlineTheme
is not getting the IgnoreTokenType
for Addition/Deletion endings, being unable to replace them with '{-' and '-}'. So I've fixed it by returning the class when it's not found in the theme file instead of returning an empty span. This also return the classes of highlighting tags in case that are not present in the theme file.
In addition to the last PR, I've removed the spacing after <span class="hl-gutter ">
if it does not follow by another class.
Also if you could explain the use of IgnoreTokenType
, disabling it even solves this InlineTheme problem and in all my testing I could not find any scenario that it's helping in any way, results being the same with or without. Probably it has a meaning, I was just wondering if it's really necessary or if we could find another solution.
P.S. Sorry for the unit-tests I always fucked them 😳
Pull Request Test Coverage Report for Build 9204549281
Warning: This coverage report may be inaccurate.
This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
- For more information on this, see Tracking coverage changes with pull request builds.
- To avoid this issue with future PRs, see these Recommended CI Configurations.
- For a quick fix, rebase this PR at GitHub. Your next report should be accurate.
Details
- 4 of 5 (80.0%) changed or added relevant lines in 4 files are covered.
- 1 unchanged line in 1 file lost coverage.
- Overall coverage increased (+0.1%) to 95.411%
Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
---|---|---|---|
src/Themes/InlineTheme.php | 0 | 1 | 0.0% |
<!-- | Total: | 4 | 5 |
Files with Coverage Reduction | New Missed Lines | % |
---|---|---|
src/Languages/Php/PhpDocCommentLanguage.php | 1 | 93.33% |
<!-- | Total: | 1 |
Totals | |
---|---|
Change from base Build 9013465686: | 0.1% |
Covered Lines: | 1497 |
Relevant Lines: | 1569 |
💛 - Coveralls
Looks good! To answer your question:
Also if you could explain the use of IgnoreTokenType
They used to be so that there aren't any collisions between blade comments {{--
and ignore tags {{-
.
However, it's more than possible that this now also works without these tokens, because of other changes that have been made. I'll look into this separately, let's keep this PR focussed on one thing :)
I'm fine merging this in, but could you please explain that one question about the regex differences?
Indeed, using '/\R/'
is more comprehensive and handles all cases in one go including unicode. Also being inlined with #115
Tagged: https://github.com/tempestphp/highlight/releases/tag/2.4.5