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

Add: enable command line command #3040

Merged
merged 26 commits into from
Feb 1, 2024
Merged

Conversation

Jim8y
Copy link
Contributor

@Jim8y Jim8y commented Dec 29, 2023

This pr add commandline commands to neo-cli:

commands:

new Option<string>(new[] { "-c", "--config","/config" }, "Specifies the config file."),
new Option<string>(new[] { "-w", "--wallet","/wallet" }, "The path of the neo3 wallet [*.json]."),
new Option<string>(new[] { "-p", "--password" ,"/password" }, "Password to decrypt the wallet, either from the command line or config file."),
new Option<string>(new[] { "--db-engine","/db-engine" }, "Specify the db engine."),
new Option<string>(new[] { "--db-path","/db-path" }, "Specify the db path."),
new Option<string>(new[] { "--noverify","/noverify" }, "Indicates whether the blocks need to be verified when importing."),
new Option<string[]>(new[] { "--plugins","/plugins" }, "The names of the plugins [plugin1 plugin2]."),

@cschuchardt88
Copy link
Member

This is starting to turn into neo-express? What is the point of this?

@Jim8y
Copy link
Contributor Author

Jim8y commented Dec 30, 2023

This is starting to turn into neo-express? What is the point of this?

Does it hurt?

* master:
  Fixed asp.net core project (neo-project#3067)
  Updated BLS12_381 (neo-project#3074)
  avoid nonsense exception messages. (neo-project#3063)
  Removed `MyGet` (neo-project#3071)
  Updated unit-test (neo-project#3073)
  add hash verification for OnImport (neo-project#3070)
  Make public ReadUserInput (neo-project#3068)
  Removed asp.net core (neo-project#3065)
  Enforce Line Endings in `.editorconfig` (neo-project#3060)
  Remove some warnings (neo-project#3057)
  Fixed workflow  timeout-minutes (neo-project#3048)
  Fix: specify the error message (neo-project#3030)
  Removes `WebSocket`s from the network layer (neo-project#3039)
  set timeout for tests (neo-project#3046)
  Fix: Editconfig (neo-project#3023)
  Set project as nullable (neo-project#3042)
  Fix: fix equal (neo-project#3028)

# Conflicts:
#	src/Neo.CLI/CLI/MainService.cs
#	src/Neo.CLI/Settings.cs
#	src/Neo/ProtocolSettings.cs
Copy link
Member

@shargon shargon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why src/Neo.CLI/CLI/CommandLineOption.cs warnings if it was not changed and we already merge some commits without this problem

shargon
shargon previously approved these changes Jan 11, 2024
Copy link
Member

@shargon shargon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any way to use /help with this package?

src/Neo.CLI/Settings.cs Show resolved Hide resolved
@shargon
Copy link
Member

shargon commented Jan 11, 2024

@Jim8y Conflicting files :S

Jim8y added 5 commits January 11, 2024 21:00
* master:
  Adding NNS to `neo-cli` (neo-project#3032)

# Conflicts:
#	src/Neo.CLI/Settings.cs
This reverts commit 7d54c5d.
@shargon
Copy link
Member

shargon commented Jan 11, 2024

You don't want to move the config files?

shargon
shargon previously approved these changes Jan 11, 2024
Copy link
Member

@vncoelho vncoelho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/workspaces/neo/src/Neo.CLI/Settings.cs(65,16): warning CS8618: Non-nullable property 'Contracts' must contain a non-null value when exiting constructor. Consider declaring the property as nullable. [/workspaces/neo/src/Neo.CLI/Neo.CLI.csproj::TargetFramework=net7.0]
/workspaces/neo/src/Neo.CLI/CLI/MainService.CommandLine.cs(79,21): error CS0200: Property or indexer 'StorageSettings.Engine' cannot be assigned to -- it is read only [/workspaces/neo/src/Neo.CLI/Neo.CLI.csproj::TargetFramework=net7.0]
/workspaces/neo/src/Neo.CLI/CLI/MainService.CommandLine.cs(80,21): error CS0200: Property or indexer 'StorageSettings.Path' cannot be assigned to -- it is read only [/workspaces/neo/src/Neo.CLI/Neo.CLI.csproj::TargetFramework=net7.0]

Build FAILED.

/workspaces/neo/src/Neo.CLI/Settings.cs(65,16): warning CS8618: Non-nullable property 'Contracts' must contain a non-null value when exiting constructor. Consider declaring the property as nullable. [/workspaces/neo/src/Neo.CLI/Neo.CLI.csproj::TargetFramework=net7.0]
/workspaces/neo/src/Neo.CLI/CLI/MainService.CommandLine.cs(79,21): error CS0200: Property or indexer 'StorageSettings.Engine' cannot be assigned to -- it is read only [/workspaces/neo/src/Neo.CLI/Neo.CLI.csproj::TargetFramework=net7.0]
/workspaces/neo/src/Neo.CLI/CLI/MainService.CommandLine.cs(80,21): error CS0200: Property or indexer 'StorageSettings.Path' cannot be assigned to -- it is read only [/workspaces/neo/src/Neo.CLI/Neo.CLI.csproj::TargetFramework=net7.0]
    1 Warning(s)
    2 Error(s)

I think that I destroyed something when merged and conflicts resolved

@Jim8y
Copy link
Contributor Author

Jim8y commented Jan 12, 2024

I dont know, my local test passes.

src/Neo.CLI/Settings.cs Show resolved Hide resolved
src/Neo.CLI/CLI/MainService.cs Outdated Show resolved Hide resolved
@shargon shargon merged commit 569b614 into neo-project:master Feb 1, 2024
2 checks passed
@cschuchardt88
Copy link
Member

You have changed the interface. In doing so, i have found a bug. As you can see from the images below. That the CLI prompt is still open. When people try to run a command like show state it will clear the screen and render nothing. Please revert!!!

Run .\neo-cli.exe /noverify

image

when you type in show state

image

@shargon why did you merge this? You need two approvals. I only see one.

for (int i = 0; i < args.Length; i++)
switch (args[i])
{
case "/noverify":
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be added back somehow.

@shargon
Copy link
Member

shargon commented Feb 11, 2024

You have changed the interface. In doing so, i have found a bug. As you can see from the images below. That the CLI prompt is still open. When people try to run a command like show state it will clear the screen and render nothing. Please revert!!!

Run .\neo-cli.exe /noverify

image

when you type in show state

image

@shargon why did you merge this? You need two approvals. I only see one.

We can add in the readme the backwards incompatible changes, but we can't preserve all the previous version if we try to improve the new one, when it's released it could come with this information. With this change, the cli is much better, and people only need to change /verify to --xxx

@shargon
Copy link
Member

shargon commented Feb 11, 2024

@shargon why did you merge this? You need two approvals. I only see one.

Is not my pr, so technically is @Jim8y and me, is two.

@cschuchardt88
Copy link
Member

You have changed the interface. In doing so, i have found a bug. As you can see from the images below. That the CLI prompt is still open. When people try to run a command like show state it will clear the screen and render nothing. Please revert!!!

Run .\neo-cli.exe /noverify

image

when you type in show state

image
@shargon why did you merge this? You need two approvals. I only see one.

We can add in the readme the backwards incompatible changes, but we can't preserve all the previous version if we try to improve the new one, when it's released it could come with this information. With this change, the cli is much better, and people only need to change /verify to --xxx

that is fine.

Can you fix the bug with the cli prompt bugging out on invalid arguments on command-line? cli command show state doesn't work anymore when that happens.

@shargon
Copy link
Member

shargon commented Feb 11, 2024

You have changed the interface. In doing so, i have found a bug. As you can see from the images below. That the CLI prompt is still open. When people try to run a command like show state it will clear the screen and render nothing. Please revert!!!

Run .\neo-cli.exe /noverify

image

when you type in show state

image
@shargon why did you merge this? You need two approvals. I only see one.

We can add in the readme the backwards incompatible changes, but we can't preserve all the previous version if we try to improve the new one, when it's released it could come with this information. With this change, the cli is much better, and people only need to change /verify to --xxx

that is fine.

Can you fix the bug with the cli prompt bugging out on invalid arguments on command-line? cli command show state doesn't work anymore when that happens.

We found a bug, we fix the bug, we can open a new pr to fix it, please open a issue, remember that this version is not released yet.

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.

5 participants