Skip to content

Fixing indentation stripped (affect diff highlighting)#168

Merged
greg0ire merged 2 commits intodoctrine:0.4.xfrom
weaverryan:diff-highlighting
Oct 21, 2021
Merged

Fixing indentation stripped (affect diff highlighting)#168
greg0ire merged 2 commits intodoctrine:0.4.xfrom
weaverryan:diff-highlighting

Conversation

@weaverryan
Copy link
Collaborator

Hi!

This fixes symfony-tools/docs-builder#124

tl;dr. Suppose you have

code-block: diff

      Normal Line
    + Added line

The code-block indentation is really 4 spaces. But because that first line (Normal Line) is indented by 6 spaces (2 of which should be part of the code block), the Node class currently thinks that the code block is indented by 6 spaces. It then strips the first 6 characters off each line, which strips the + in front of + Added line.

Cheers!

$rendered = $document->$renderMethod();

if ($format === Format::HTML) {
if ($format === Format::HTML && $useIndenter) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is the ugliest part of the PR: using the Indenter really made a mess of the HTML - it was removing some line breaks in odd places. Instead of making the HTML file match what the Indenter wanted, I removed it for this test. It makes it clear that we're getting the exact HTML we want.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ref gajus/dindent#28 (maybe we can somehow reactive that PR?)

Copy link
Collaborator

@wouterj wouterj left a comment

Choose a reason for hiding this comment

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

Looks good

$rendered = $document->$renderMethod();

if ($format === Format::HTML) {
if ($format === Format::HTML && $useIndenter) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ref gajus/dindent#28 (maybe we can somehow reactive that PR?)

@greg0ire greg0ire merged commit c47f155 into doctrine:0.4.x Oct 21, 2021
@greg0ire greg0ire added this to the 0.4.4 milestone Oct 21, 2021
@greg0ire
Copy link
Member

Thanks @weaverryan !

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.

3 participants