Skip to content

Fixing indentation stripped (affect diff highlighting) #168

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.

Sign up for GitHub

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

Merged
greg0ire merged 2 commits into from
Oct 21, 2021
Merged

Fixing indentation stripped (affect diff highlighting) #168

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bb3d9cf
Adding failing test case
weaverryan Oct 20, 2021
0875723
Making indentation stripping smarter by checking all lines
weaverryan Oct 20, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 14 additions & 6 deletions lib/Nodes/Node.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
use Doctrine\RST\Renderers\RenderedNode;

use function implode;
use function ltrim;
use function strlen;
use function substr;
use function trim;
Expand Down Expand Up @@ -137,18 +138,25 @@ public function getValueString(): string
protected function normalizeLines(array $lines): string
{
if ($lines !== []) {
$firstLine = $lines[0];
$indentLevel = null;

$k = 0;
// find the indentation by locating the line with the fewest preceding whitespace
foreach ($lines as $line) {
// skip empty lines
if (trim($line) === '') {
continue;
}

for ($k = 0; $k < strlen($firstLine); $k++) {
if (trim($firstLine[$k]) !== '') {
break;
$startingWhitespace = strlen($line) - strlen(ltrim($line));
if ($indentLevel !== null && $startingWhitespace > $indentLevel) {
continue;
}

$indentLevel = $startingWhitespace;
}

foreach ($lines as &$line) {
$line = substr($line, $k);
$line = substr($line, $indentLevel);
}
}

Expand Down
10 changes: 7 additions & 3 deletions tests/Functional/FunctionalTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
class FunctionalTest extends TestCase
{
private const RENDER_DOCUMENT_FILES = ['main-directive'];
private const SKIP_INDENTER_FILES = ['code-block-diff'];

protected function setUp(): void
{
Expand All @@ -47,7 +48,8 @@ public function testFunctional(
string $renderMethod,
string $format,
string $rst,
string $expected
string $expected,
bool $useIndenter = true
): void {
$expectedLines = explode("\n", $expected);
$firstLine = $expectedLines[0];
Expand All @@ -68,7 +70,7 @@ public function testFunctional(

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

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

@weaverryan weaverryan Oct 20, 2021

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

@wouterj wouterj Oct 20, 2021

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?)

$indenter = new Indenter();
$rendered = $indenter->indent($rendered);
}
Expand Down Expand Up @@ -133,7 +135,9 @@ public function getFunctionalTests(): array
? 'renderDocument'
: 'render';

$tests[$basename . '_' . $format] = [$basename, $parser, $renderMethod, $format, $rst, trim($expected)];
$useIndenter = ! in_array($basename, self::SKIP_INDENTER_FILES, true);

$tests[$basename . '_' . $format] = [$basename, $parser, $renderMethod, $format, $rst, trim($expected), $useIndenter];
}
}

Expand Down
14 changes: 14 additions & 0 deletions tests/Functional/tests/code-block-diff/code-block-diff.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<pre><code class="diff">+ Added line
- Removed line
Normal line
- Removed line
+ Added line

</code></pre>
<pre><code class="diff"> Normal line
+ Added line
- Removed line
Normal line
- Removed line
+ Added line
</code></pre>
18 changes: 18 additions & 0 deletions tests/Functional/tests/code-block-diff/code-block-diff.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@

.. code-block:: diff

+ Added line
- Removed line
Normal line
- Removed line
+ Added line


.. code-block:: diff

Normal line
+ Added line
- Removed line
Normal line
- Removed line
+ Added line