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

Invalid auto-generated markdown for procedure parameters wrapped in double-quotes #399

Open
vody opened this issue Jul 7, 2022 · 6 comments
Labels
bug Something isn't working

Comments

@vody
Copy link
Contributor

vody commented Jul 7, 2022

Describe the bug
With procedure parameters wrapped in double quotes as in the example below extension generates an invalid markdown.
Code Example:

procedure GuidToUUID("Guid": Guid): Text
begin
    exit(Format("Guid", 0, 4).ToLower())
end;

Autogenerated Markdown:

## <a name="parameters"></a>Parameters

### <a name=""Guid""></a>`"Guid"`  Guid

Versions used
Version: 1.68.1 (user setup)
Commit: 30d9c6cd9483b2cc586687151bcbcd635f373630
Date: 2022-06-14T12:48:58.283Z
Electron: 17.4.7
Chromium: 98.0.4758.141
Node.js: 16.13.0
V8: 9.8.177.13-electron.0
OS: Windows_NT x64 10.0.19044
NAB AL Tools: v1.27.207070852

To Reproduce
Steps to reproduce the behavior:

  1. Add codeunit with procedure from example which has any parameter wrapped in double-quotes
  2. Run a NAB: Generate External Documentation command
  3. Open a generated markdown for the newly created procedure
  4. See invalid markdown generate for parameters section

Expected behavior
Double-quotes should be removed before the parameter name is used to generate markdown content. Example:

## <a name="parameters"></a>Parameters

### <a name="Guid"></a>`Guid`  Guid
@vody vody added the bug Something isn't working label Jul 7, 2022
vody added a commit to vody/nab-al-tools that referenced this issue Jul 7, 2022
@jwikman
Copy link
Owner

jwikman commented Jul 7, 2022

Hi again @vody, and thanks for reporting this.
We've banned quotes for variables and parameters, and they can easily be workaround just by giving them another name. :)

That being said, I understand that there are old code that you might not be able to change. At least not right now.

I'm not sure that I agree with your suggestion. If it is named with double quotes, why not output that with double quotes? Leaving them out of the anchor name, since otherwise it is invalid.
Like this:

### <a name="Guid"></a>`"Guid"`  Guid

If this should be fixed, scenarios as below should be handled as well.

procedure MyProcedure("A variable with spaces": Guid): Text
begin
    //
end;

it should result in something like this, right?

### <a name="a_variable_with_spaces"></a>`"A variable with spaces"`  Guid

What do you say about that?

@vody
Copy link
Contributor Author

vody commented Jul 7, 2022

@jwikman, an issue has been raised as we have a valid AL code and an invalid markdown one. :)
From a procedure usage point of view, the user (developer) will not see a difference if it's double-quoted or not. So don't see any value in keeping double quotes in the documentation. Fully agree that we need to keep spaces and replace spaces with underscores to keep parameter references as straightforward as possible.
Please confirm if you like this approach so I can update my PR accordingly.

@vody
Copy link
Contributor Author

vody commented Jul 8, 2022

@jwikman, if we would like to bring this change in, then we may need to apply the same logic for returns.

@jwikman
Copy link
Owner

jwikman commented Jul 8, 2022

@vody I added some comments in the PR :)

I'm fine with removing the " from the output in docs, but when we're at it, we should probably support valid names with double double-quotes inside the names as well.
So a parameter called "Name with ""Quotes""" should be output as Name with "Quotes" in the docs.

Don't you agree?

@vody
Copy link
Contributor Author

vody commented Jul 10, 2022

@jwikman, agree. I will try to find some time to redo a PR.

@jwikman
Copy link
Owner

jwikman commented Jul 10, 2022

@vody Just reach out to me or @theschitz if you need any help (or if you just can't find the time for this). However, the time zone difference may cause some response delay... 😁

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants