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: Invalid auto-generated markdown for procedure parameters wrapped in double-quotes #400

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

vody
Copy link
Contributor

@vody vody commented Jul 7, 2022

Related to #399.
Changes proposed in this pull request: Removing any double-quotes from the parameter name

vody added 5 commits July 8, 2022 09:59
Syntax fixed and add's a URL space replacement
prettier should now accept this
@theschitz
Copy link
Collaborator

Please format src/Documentation.ts with prettier :)

@theschitz theschitz requested a review from jwikman July 8, 2022 07:56
@jwikman
Copy link
Owner

jwikman commented Jul 8, 2022

As @theschitz says, format your code with prettier and you'll have better luck with the checks. :)
Install the recommended extensions to get prettier installed:
image

Some thoughts about this:

If we want this functionality to be complete, there are several (strange) names with double quotes to support:

  • "Guid": Guid
  • "Name with spaces": Guid
  • "Name with ""Quotes""": Guid
  • Others?

Instead of adding the replace statement inside Documentation.ts, I would suggest adding a couple of functions to src\ALObject\ALVariable.ts, just to keep same structure as for src\ALObject\ALProcedure.ts:

public get docsAnchor(): string {
  return `${snakeCase(this.name)}`;
}
public get docsName(): string {
  // Add logic to:
  // - Remove surrounding double quotes
  // - Replace "" inside name with "
}

And then call those functions instead of the name property.

To test your changes (and visualize for me), please update the TestApp:

  • Open TestApp from debugger pane:
    image
  • Add procedures that has parameters and return variable names that are affected by your change (create different combinations, with and without spaces etc). Add them in a new public codeunit, or add to an existing one (some changes might make other tests to fail, but you'll notice 😉).
  • Run the NAB: Generate External Documentation command to update the docs and check the results.
  • When you are happy with your changes, you can run "All Automatic Tests" from the debugger pane. In that way you do not need to wait for the GitHub Actions to run when updating your PR.
  • Commit and push changes of TestApp (AL code + Docs .md files) to the PR as well.

Thanks for your effort in contributing to this repo! :)

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.

None yet

3 participants