Skip to content

[Event] Add EVENT_CLI_SUCCESSFUL_EXECUTE #224

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.

Sign up for GitHub

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

Merged
jiasli merged 3 commits into from
Dec 16, 2020
Merged

[Event] Add EVENT_CLI_SUCCESSFUL_EXECUTE #224

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8b3757d
Add EVENT_CLI_SUCCESSFUL_EXECUTE
jiasli Oct 13, 2020
61057a3
Send cmd_result.result
jiasli Dec 11, 2020
61c3f88
Merge branch 'master' into hint
jiasli Dec 16, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion knack/cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
from .util import CLIError
from .config import CLIConfig
from .query import CLIQuery
from .events import EVENT_CLI_PRE_EXECUTE, EVENT_CLI_POST_EXECUTE
from .events import EVENT_CLI_PRE_EXECUTE, EVENT_CLI_SUCCESSFUL_EXECUTE, EVENT_CLI_POST_EXECUTE
from .parser import CLICommandParser
from .commands import CLICommandsLoader
from .help import CLIHelp
Expand Down Expand Up @@ -219,6 +219,7 @@ def invoke(self, args, initial_invocation_data=None, out_file=None):
if cmd_result and cmd_result.result is not None:
formatter = self.output.get_formatter(output_type)
self.output.out(cmd_result, formatter=formatter, out_file=out_file)
self.raise_event(EVENT_CLI_SUCCESSFUL_EXECUTE, result=cmd_result.result)
Copy link

@fengzhou-msft fengzhou-msft Dec 14, 2020

Choose a reason for hiding this comment

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

Should we raise the event only when exit_code is 0?

Copy link
Member Author

@jiasli jiasli Dec 14, 2020

Choose a reason for hiding this comment

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

Yes. On the other hand, EVENT_CLI_POST_EXECUTE is raised unconditionally.

knack/knack/cli.py

Lines 233 to 234 in 61057a3

finally:
self.raise_event(EVENT_CLI_POST_EXECUTE)

Copy link

@fengzhou-msft fengzhou-msft Dec 14, 2020

Choose a reason for hiding this comment

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

Then should you check the exit_code from

knack/knack/cli.py

Line 217 in 61057a3

exit_code = self.result.exit_code

Copy link
Member Author

@jiasli jiasli Dec 15, 2020

Choose a reason for hiding this comment

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

Not really necessary.

knack/knack/cli.py

Line 215 in 61057a3

cmd_result = self.invocation.execute(args)

will raise an exception first if exit_code is not 0.

Also,

knack/knack/cli.py

Line 221 in 61057a3

self.output.out(cmd_result, formatter=formatter, out_file=out_file)

already assumes the command is successfully executed and exit_code is 0.

Copy link

@fengzhou-msft fengzhou-msft Dec 15, 2020

Choose a reason for hiding this comment

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

got it.

except KeyboardInterrupt as ex:
exit_code = 1
self.result = CommandResultItem(None, error=ex, exit_code=exit_code)
Expand Down
1 change: 1 addition & 0 deletions knack/events.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
# --------------------------------------------------------------------------------------------

EVENT_CLI_PRE_EXECUTE = 'Cli.PreExecute'
EVENT_CLI_SUCCESSFUL_EXECUTE = 'Cli.SuccessfulExecute'
EVENT_CLI_POST_EXECUTE = 'Cli.PostExecute'

EVENT_INVOKER_PRE_CMD_TBL_CREATE = 'CommandInvoker.OnPreCommandTableCreate'
Expand Down