microsoft / go-sqlcmd

The new sqlcmd, CLI for SQL Server and Azure SQL (winget install sqlcmd / sqlcmd create mssql / sqlcmd open ads)
https://learn.microsoft.com/sql/tools/sqlcmd/go-sqlcmd-utility
MIT License
325 stars 57 forks source link

usage formatting #360

Closed JyotikaGargg closed 1 year ago

stuartpa commented 1 year ago

There is a bug for flags that do not have a short cut, they print out as just a "-":

-, --version Print version information and exit

-, --driver-logging-level Level of mssql driver messages to print.

stuartpa commented 1 year ago

Kubectl is doing a paragraph align of the descriptions, e.g.

Options:
    -c, --container='':
        Container name. If omitted, use the kubectl.kubernetes.io/default-container annotation for
        selecting the container to be attached or the first container in the pod will be chosen

    --no-preserve=false:
        The copied file/directory's ownership and permissions will not be preserved in the
        container

Where as this PR is wrapping to col 1, e.g.

  -G, --use-aad
      Tells sqlcmd to use Active Directory authentication. If no user name is provided, authentication method ActiveDirectoryDefault is used. If a password is provided, ActiveDirectoryPassword is used. Otherwise ActiveDirectoryInteractive is used.

  -E, --use-trusted-connection
      Uses a trusted connection instead of using a user name and password to sign in to SQL Server, ignoring any environment variables that define user name and password.

  -U, --user-name
      The login name or contained database user name.  For contained database users, you must provide the database name option

Can we align the description text as well please.

stuartpa commented 1 year ago

Given the sheer number of flags, it would be nice to be able to put the flags into groups. It looks like Cobra doesn't support this yet, this PR would need to be merged:

https://github.com/spf13/cobra/pull/1778

Is there any other easy way to provide flag grouping that you know of?

JyotikaGargg commented 1 year ago

Updated the usage formatting similar to kubectl: -d, --database-name --This option sets the sqlcmd scripting variable SQLCMDDBNAME. This parameter specifies the initial database. The default is your login's default-database property. If the database does not exist, an error message is generated and sqlcmd exits.

-X, --disable-cmd-and-warn --Disables commands that might compromise system security. Sqlcmd issues a warning and continues.

JyotikaGargg commented 1 year ago

There is a bug for flags that do not have a short cut, they print out as just a "-":

-, --version Print version information and exit

-, --driver-logging-level Level of mssql driver messages to print.

Resolved this.

stuartpa commented 1 year ago

There is a bug for flags that do not have a short cut, they print out as just a "-": -, --version Print version information and exit -, --driver-logging-level Level of mssql driver messages to print.

Resolved this.

Looks much better:

-c, --batch-terminator --Specifies the batch terminator. The default value is GO

-s, --column-separator --Specifies the column separator character. Sets the SQLCMDCOLSEP variable.

There is an extra "--" at the beginning of each description...

stuartpa commented 1 year ago

Also, while you here, can you fix:

  --Specifies the SQL authentication method to use to connect
    to Azure SQL Database. One
of:ActiveDirectoryDefault,ActiveDirectoryIntegrated,ActiveDirectoryPassword,ActiveDirectoryInteractive,ActiveDirectoryManagedIdentity,ActiveDirectoryServicePrincipal,SqlPassword

So there are some spaces in the string to it can wrap correctly.

JyotikaGargg commented 1 year ago

Also, while you here, can you fix:

  --Specifies the SQL authentication method to use to connect
    to Azure SQL Database. One
of:ActiveDirectoryDefault,ActiveDirectoryIntegrated,ActiveDirectoryPassword,ActiveDirectoryInteractive,ActiveDirectoryManagedIdentity,ActiveDirectoryServicePrincipal,SqlPassword

So there are some spaces in the string to it can wrap correctly.

Resolved

JyotikaGargg commented 1 year ago

There is a bug for flags that do not have a short cut, they print out as just a "-": -, --version Print version information and exit -, --driver-logging-level Level of mssql driver messages to print.

Resolved this.

Looks much better:

-c, --batch-terminator --Specifies the batch terminator. The default value is GO

-s, --column-separator --Specifies the column separator character. Sets the SQLCMDCOLSEP variable.

There is an extra "--" at the beginning of each description...

Resolved

stuartpa commented 1 year ago

This is looking much much better. Nit: can you bring the help description back to spaces, so it is the same as kubectl, e.g.:

''' Options: -c, --container='': Container name. If omitted, use the kubectl.kubernetes.io/default-container annotation for selecting the container to be attached or the first container in the pod will be chosen

--no-preserve=false:
    The copied file/directory's ownership and permissions will not be preserved in the
    container
JyotikaGargg commented 1 year ago

@stuartpa Are you talking about the description should look like below :

Flags:
-K,--application-intent
Declares the application workload type when connecting to a
server. The only currently supported value is ReadOnly. If
-K is not specified, the sqlcmd utility will not support
connectivity to a secondary replica in an Always On
availability group

      --authentication-method
Specifies the SQL authentication method to use to connect
to Azure SQL Database. One of: ActiveDirectoryDefault,
ActiveDirectoryIntegrated, ActiveDirectoryPassword,
ActiveDirectoryInteractive, ActiveDirectoryManagedIdentity,
ActiveDirectoryServicePrincipal, SqlPassword

-c,--batch-terminator
Specifies the batch terminator. The default value is GO

beacuse right now it looks like this :

Flags:
  -K, --application-intent
        Declares the application workload type when connecting to a
        server. The only currently supported value is
        ReadOnly. If -K is not specified, the sqlcmd
        utility will not support connectivity to a
        secondary replica in an Always On availability
        group

      --authentication-method
        Specifies the SQL authentication method to use to connect
        to Azure SQL Database. One of:
        ActiveDirectoryDefault, ActiveDirectoryIntegrated,
        ActiveDirectoryPassword,
        ActiveDirectoryInteractive,
        ActiveDirectoryManagedIdentity,
        ActiveDirectoryServicePrincipal, SqlPassword

  -c, --batch-terminator
        Specifies the batch terminator. The default value is GO

  -s, --column-separator
        Specifies the column separator character. Sets the
        SQLCMDCOLSEP variable.

I am liking lower one better as compared to first one. But still if we want to have flags without spaces , i could do those changes.

stuartpa commented 1 year ago

@stuartpa Are you talking about the description should look like below :

Flags:
-K,--application-intent
Declares the application workload type when connecting to a
server. The only currently supported value is ReadOnly. If
-K is not specified, the sqlcmd utility will not support
connectivity to a secondary replica in an Always On
availability group

      --authentication-method
Specifies the SQL authentication method to use to connect
to Azure SQL Database. One of: ActiveDirectoryDefault,
ActiveDirectoryIntegrated, ActiveDirectoryPassword,
ActiveDirectoryInteractive, ActiveDirectoryManagedIdentity,
ActiveDirectoryServicePrincipal, SqlPassword

-c,--batch-terminator
Specifies the batch terminator. The default value is GO

beacuse right now it looks like this :

Flags:
  -K, --application-intent
        Declares the application workload type when connecting to a
        server. The only currently supported value is
        ReadOnly. If -K is not specified, the sqlcmd
        utility will not support connectivity to a
        secondary replica in an Always On availability
        group

      --authentication-method
        Specifies the SQL authentication method to use to connect
        to Azure SQL Database. One of:
        ActiveDirectoryDefault, ActiveDirectoryIntegrated,
        ActiveDirectoryPassword,
        ActiveDirectoryInteractive,
        ActiveDirectoryManagedIdentity,
        ActiveDirectoryServicePrincipal, SqlPassword

  -c, --batch-terminator
        Specifies the batch terminator. The default value is GO

  -s, --column-separator
        Specifies the column separator character. Sets the
        SQLCMDCOLSEP variable.

I am liking lower one better as compared to first one. But still if we want to have flags without spaces , i could do those changes.

Like the lower one, but back two spaces