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

Incorrect help text in CLI page #8535

Closed
anupama-pathirage opened this issue Jan 16, 2024 · 9 comments
Closed

Incorrect help text in CLI page #8535

anupama-pathirage opened this issue Jan 16, 2024 · 9 comments
Assignees
Labels
Area/LearnPages Issues related to all the pages of the Learn section. Reason/EngineeringMistake Reason/EngineeringMistake The issue occurred due to a mistake made in the past. Type/Bug Bugs that need to be fixed.

Comments

@anupama-pathirage
Copy link
Contributor

anupama-pathirage commented Jan 16, 2024

Description

$subject in https://ballerina.io/learn/cli-commands/#use-the-ballerina-tool

  1. There is additional command named tool which is not there in the output. @azinneera it seems this command is not displayed as Other command in help text. Is that correct? If correct, we need to update the docs.
image
  1. EDI command is not there.
image
@azinneera
Copy link
Contributor

azinneera commented Jan 16, 2024

  1. EDI command is not there.
image

We moved the tool to the Other Commands section. We have missed to update the website doc.

About the EDI Command - we didn't add it to the website since it will be shown only if the user has installed it. We might have to add an additional doc to show how the help text works for a custom tool installation.

@anupama-pathirage
Copy link
Contributor Author

@azinneera, is it correct to have the edi command listed here in the help text? All the commands listed here are the ones which comes after bal. But edi comes after bal tool. And it is a custom tool command which is downloaded from central. So is it correct to to have it here as first level of commands here? Same applies to health command.

@gayaldassanayake
Copy link
Contributor

gayaldassanayake commented Jan 16, 2024

Tool being listed under core commands in the website indeed is a mistake. Will fix right away.

However listing edi in the first level was a conscious decision since once edi tool (or any other tool installed with bal tool) is installed with bal tool pull edi, the user can use the edi tool with bal edi (and not bal tool edi) similar to any natively provided CLI command.

@anupama-pathirage
Copy link
Contributor Author

I get the empty output when issuing bal help edi. Shouldn't we have help text for edi tool in that case?

Note: I have already installed edi tool.

image

@gayaldassanayake
Copy link
Contributor

gayaldassanayake commented Jan 16, 2024

The printLongDesc(StringBuilder out) method that is provided with BLauncherCmd interface should append the help text to the out string builder. However, the method body is empty now.
https://github.com/ballerina-platform/edi-tools/blob/60ef628b6e59efea1c4efb12f338c3bf8735b98c/edi-cli/src/main/java/io/ballerina/edi/cmd/EdiCmd.java#L87

This section of the bal tool startup guide explains this.
https://docs.google.com/document/d/19ptscl5kcmvAmzztUv8EAhsx13DUdLIJanlZt2BsZG8/edit#heading=h.r9i8aegmayof

So the method should be as below.

@Override
    public void printLongDesc(StringBuilder out) {
        stringBuilder.append("Ballerina EDI tools -\n");
        stringBuilder.append("Ballerina code generation for edi schema: bal edi codegen -s <schema json path> -o <output bal file path>\n");
        stringBuilder.append("EDI library generation: bal edi libgen -O <org name> -l <library name> -s <EDI schema folder> -o <output folder>\n");
        stringBuilder.append("ESL to Ballerina EDI schema conversion: bal edi convertESL -b <Segment definitions file path> -s <ESL schema file/folder> -o <output file/folder>\n");
        stringBuilder.append("Ballerina X12 schema conversion: bal edi codegen [-H] [-c] -i <schema input path> -o <output json file/folder path> [-d] <segment details path>\n");
    }

@niveathika

@niveathika
Copy link
Contributor

@RDPerera Lets look through and fix all possible edit text.

@RDPerera
Copy link
Member

RDPerera commented Jan 17, 2024

@RDPerera Lets look through and fix all possible edit text.

Fix the issues with 0.8.6 release from tool side. In the meantime, I am working on improving the help text of the EDI tool to match standard Ballerina CLI help texts as well.

Screenshot 2024-01-17 at 10 48 33

@praneesha
Copy link

Fixed via #8538.

Copy link

This issue is NOT closed with a proper Reason/ label. Make sure to add proper reason label before closing. Please add or leave a comment with the proper reason label now.

      - Reason/EngineeringMistake - The issue occurred due to a mistake made in the past.
      - Reason/Regression - The issue has introduced a regression.
      - Reason/MultipleComponentInteraction - Issue occured due to interactions in multiple components.
      - Reason/Complex - Issue occurred due to complex scenario.
      - Reason/Invalid - Issue is invalid.
      - Reason/Other - None of the above cases.

@praneesha praneesha added the Reason/EngineeringMistake Reason/EngineeringMistake The issue occurred due to a mistake made in the past. label Jan 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area/LearnPages Issues related to all the pages of the Learn section. Reason/EngineeringMistake Reason/EngineeringMistake The issue occurred due to a mistake made in the past. Type/Bug Bugs that need to be fixed.
Projects
None yet
Development

No branches or pull requests

6 participants