codefori / vscode-rpgle

RPGLE language tools for VS Code
MIT License
39 stars 20 forks source link

ILEDocs procedure documentation hover text not as expected #202

Closed richardm90 closed 1 year ago

richardm90 commented 1 year ago

I've started using the ILEDocs format for documenting procedures but have noticed the hover text doesn't include all of the information included in the documentation or at least not as I was expecting.

Taking the basic example from the documentation.

///
// Transform to lowercase
// This procedure will take a string and transform it to lowercase
//
// @param The string
// @return The lowercase value
///
Dcl-Proc ToLower Export;
  Dcl-Pi *N Char(20);
    stringIn Char(20);
  End-pi;

  return STRLOWER(stringIn);
End-Proc;

This shows as follows, in the hover text.

image

The title is not shown and neither is the @param description.

Is this behaviour intended?


Variants

{
  "american": "%23@$",
  "local": "%23@£"
}

Errors:

[
  {
    "command": "system \"CHKOBJ OBJ(QSYS/QCPFRMIMPF) OBJTYPE(*DTAARA)\"",
    "code": 255,
    "stderr": "CPF9801: Object QCPFRMIMPF in library QSYS not found.",
    "cwd": "/home/richardm"
  },
  {
    "command": "ls -p /QSYS.lib/RICHARD.lib/GENCMDXML.PGM",
    "code": 2,
    "stderr": "/QSYS.lib/RICHARD.lib/GENCMDXML.PGM not found",
    "cwd": "/home/richardm"
  },
  {
    "command": "/QOpenSys/usr/bin/qsh",
    "code": 1,
    "stderr": "CPF3232:  File RG%23A659057 in library RICHARD already exists.\nCPF3066:  Error creating output file RG%23A659057 in RICHARD.",
    "cwd": "/home/richardm"
  }
]
worksofliam commented 1 year ago

@richardm90 nope, but I can take a look at this. Thanks

worksofliam commented 1 year ago

@richardm90 Some further digging:

worksofliam commented 1 year ago

Looks like the fix was simple. I think at some point in the past we changed how we store the tags.

image
richardm90 commented 1 year ago

Looking good.

Personally I think it's a shame the title is not shown in the hover text. Is the logic to simply exclude the first line of the documentation block? If you have a multi-line documentation block would all of this be included (except the first line). Just trying to think about how I will use this to make the most of hover text. I couldn't see anywhere in the documentation that mentions the title is excluded but I've probably not found the right page. What elements of the documentation are included (or excluded) in the hover text?

worksofliam commented 1 year ago

Is the logic to simply exclude the first line of the documentation block?

I think it's because there was a time when we did include the title, and then the hover block was just massive.

I couldn't see anywhere in the documentation that mentions the title is excluded but I've probably not found the right page. What elements of the documentation are included (or excluded) in the hover text?

Here is the page on our support for ILEDoc. In fact, we don't document that we don't show the title at all. We probably should.

https://halcyon-tech.github.io/docs/#/pages/developing/iledocs?id=available-tags

Sounds like we need an additional issue for the title to be shown as this one is due to be closed when this current fix goes live.

BrianGodsend commented 6 months ago

I just spent some time re-discovering that the TITLE is silently dropped (and still not documented).

I do not understand the reasoning here. The TITLE is a single line of text. How can the inclusion of 1 line of text (okay 2 lines of text if you add a blank line before or after) make the hover text massive?

I often just document my subprocedures with a brief title and a description of the parameter(s) and return value. The absence of the title makes the documentation far less readable.

Would it be possible to add a configuration option to allow the inclusion or exclusion of the TITLE in the ILEDocs? This would allow the user to adapt the hover text to their usage of ILEDocs.

worksofliam commented 6 months ago

@BrianGodsend Yeah, we could look into adding the title back.

BrianGodsend commented 6 months ago

@BrianGodsend Yeah, we could look into adding the title back.

THANK YOU for looking into this ... and such a quick response on a closed issue too!