Separate handling of internal tags #661
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Julow
commented
Apr 13, 2021
| @@ -4,6 +4,8 @@ top-comment. | |||
| The module Test__X is expected to be referenced through Test.X. | |||
|
|
|||
| $ compile test__x.mli test.ml | |||
| File "test.ml", line 15, characters 6-24: | |||
| Unexpected tag '@canonical' at this location. | |||
There was a problem hiding this comment.
This is expected. This @canonical tag is indeed unhandled.
This PR implements the required refactoring to fix this easily, though.
Comment on lines
-212
to
-215
| Error.warning status.warnings e; | ||
| let placeholder = [ `Word "@canonical"; `Space " "; `Code_span s ] in | ||
| let placeholder = List.map (Location.at location) placeholder in | ||
| Error (Location.at location (`Paragraph placeholder))) |
There was a problem hiding this comment.
I've removed this placeholder logic. This tag isn't intended to be rendered. If it contains a syntax error, only the programmer need to be informed (through a warning) and not users reading the documentation.
jonludlam
reviewed
Apr 13, 2021
Internal tags are used to change Odoc's behavior and are never rendered (@canonical, @inline, @OPEN and @closed). This handles them earlier and removes them from the description of comments.
Handle tags as soon as possible instead of passing them around. This ensures that internal tags are always handled.
It was possible to attach those tags to anything and would be ignored if unecessary. This emits a warning in this case.
jonludlam
reviewed
Apr 13, 2021
|
Other than that minor point, I think this is a really nice change. |
Julow
commented
Apr 14, 2021
test/xref2/module_preamble.t/run.t
Outdated
Comment on lines
16
to
17
| File "a.mli", line 4, characters 4-17: | ||
| Unexpected tag '@canonical' at this location. |
There was a problem hiding this comment.
This isn't expected however. This is caused by `Root paths to not be valid canonical paths, I'll add a different warning for this case.
There was a problem hiding this comment.
I've just added a specific warning for this case.
This case was ignored until now. Since the warning about unexpected tag was added, this case is reported incorrectly.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #632 (comment)
Internal tags are used to change Odoc's behavior and are never rendered. This caused Stdlib modules to have no synopsis because the
@canonicaltag was found before any synopsis could be extracted.This PR handles these tags (
@canonical,@inline,@openand@closed) earlier in the pipeline and removes them from the model.This isn't the only solution to this problem but I think it's a nice refactoring to have. I believe regular tags should also be handled (previous discussion).