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 bugzilla issue 24871 - DDoc strips indent in triple slash comments #17082

Merged
merged 3 commits into from
Nov 25, 2024

Conversation

ArthaTi
Copy link
Contributor

@ArthaTi ArthaTi commented Nov 22, 2024

Removes code in the lexer responsible for removing leading spaces in triple slash doc comments.

This affects the test file for issue 14413; two more spaces appear, but they are not significant in the HTML output.

@dlang-bot
Copy link
Contributor

Thanks for your pull request and interest in making D better, @ArthaTi! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

  • My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
  • My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
  • I have provided a detailed rationale explaining my changes
  • New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.


If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Auto-close Bugzilla Severity Description
24871 trivial DDoc strips indent in triple slash comments

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#17082"

Removes code in the lexer responsible for removing leading spaces in
triple slash doc comments.

This affects the test file for issue 14413; two more spaces appear, but
they are not significant in the HTML output.
@ArthaTi
Copy link
Contributor Author

ArthaTi commented Nov 23, 2024

There's a few CI failures. I do not understand the logs from Azure pipelines. The buildkite error is from exactly the same mismatch that happens in compiler/test/compilable/extra-files/ddoc14413.html, just in DDox's test suite. I don't know what to do about this.

dkorpel added a commit to dlang/ddox that referenced this pull request Nov 23, 2024
@dkorpel
Copy link
Contributor

dkorpel commented Nov 23, 2024

Three options are:

Fixing DDox is nontrivial because when you change the expected output, it will fail as long as it's using an older DMD, so you need to make the test pass with dmd before and after this PR. Removing the test in the buildkite script works as a quick hack, but it's not ideal.

Maybe you can still strip 1 space after ///? I think usually people write:

/// Lorem
/// Ipsum

Rather than:

///Lorem
///Ipsum

@dlang-bot dlang-bot merged commit 949cb11 into dlang:master Nov 25, 2024
41 checks passed
@ArthaTi ArthaTi deleted the fix-24871 branch November 25, 2024 08:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants