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

Support documenting http responses, property list values, parameters, etc. with spaces in the name #1017

Merged

Conversation

d-ronnqvist
Copy link
Contributor

Bug/issue #, if applicable: rdar://134705941

Summary

This updates the parsing for "tagged" list (http responses, property list values, parameters, etc.) to support documenting responses/values/keys/parameters with spaces in the name.

For example:

- DictionaryKey some key with spaces: Some description of this dictionary key.

now parses the entire "some key with spaces" as the name of the dictionary key that the documentation applies to.

Dependencies

None.

Testing

The hardest part of testing this is using a symbol graph file with list elements with spaces in the names. There is one such example among the test bundles.

  • Add a new documentation extension file to Tests/S with the following content:

    # ``DictionaryData/Genre``
    
    A musical genre.
    
    - PossibleValue Classic Rock: Some description of the classic rock genre.
    - PossibleValue Folk: Some description of the folk genre.
  • Run swift run docc preview /Tests/SwiftDocCTests/Test\ Bundles/DictionaryData.docc and navigate to the "Genre" page.

    • Both the "Classic Rock" and "Folk" values should display their possible-value description
    Screenshot 2024-09-02 at 15 00 23

Checklist

Make sure you check off the following items. If they cannot be completed, provide a reason.

  • Added tests
  • Ran the ./bin/test script and it succeeded
  • [ ] Updated documentation if necessary

@d-ronnqvist
Copy link
Contributor Author

@swift-ci please test


for whitespace in [" ", " ", "\t"] {
let parameters = extractedTags("""
- Parameter\(whitespace)some parameter with spaces: Some description of this parameter.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: It looks like we're missing some tests for when there's nothing after the list item tag (e.g. just - Parameter: Some description of this parameter., would it be worth adding some?

Copy link
Contributor Author

@d-ronnqvist d-ronnqvist Sep 2, 2024

Choose a reason for hiding this comment

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

I have tests like that on lines 70-74:

XCTAssert(extractedTags("- Parameter: Missing parameter name.").parameters.isEmpty)
XCTAssert(extractedTags("- Parameter  : Missing parameter name.").parameters.isEmpty)

XCTAssert(extractedTags("- DictionaryKey: Missing key name.").parameters.isEmpty)
XCTAssert(extractedTags("- PossibleValue: Missing value name.").parameters.isEmpty)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Although now, that I read those tests I see that I'm verifying the wrong lists for the last two tests.

Copy link
Contributor Author

@d-ronnqvist d-ronnqvist Sep 2, 2024

Choose a reason for hiding this comment

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

Although now, that I read those tests I see that I'm verifying the wrong lists for the last two tests.

Fixed in 093ed59

XCTAssert(extractedTags("- Parameter: Missing parameter name.").parameters.isEmpty)
XCTAssert(extractedTags("- Parameter  : Missing parameter name.").parameters.isEmpty)

XCTAssert(extractedTags("- DictionaryKey: Missing key name.").dictionaryKeys.isEmpty)
XCTAssert(extractedTags("- PossibleValue: Missing value name.").possiblePropertyListValues.isEmpty)

Copy link
Contributor

@anferbui anferbui left a comment

Choose a reason for hiding this comment

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

LGTM, with minor suggestion

@d-ronnqvist
Copy link
Contributor Author

@swift-ci please test

@d-ronnqvist
Copy link
Contributor Author

@swift-ci please test

@d-ronnqvist d-ronnqvist merged commit 9f1c9d8 into swiftlang:main Sep 2, 2024
2 checks passed
@d-ronnqvist d-ronnqvist deleted the spaces-in-documented-dictionary-keys branch September 2, 2024 16:35
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