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
Jiashuo Li (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 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8b3757d
Add EVENT_CLI_SUCCESSFUL_EXECUTE
Oct 13, 2020
61057a3
Send cmd_result.result
Dec 11, 2020
61c3f88
Merge branch 'master' into hint
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)

@Juliehzl Zunli Hu (Juliehzl) Dec 11, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

two concerns:

  1. could the new event bring benefit to CLI if exit code = 0 could represent successful execution?
  2. should SUCCESSFUL_EXECUTE be raised after cmd_result is returned? It seems that in current code output format failure would block the event.

@jiasli Jiashuo Li (jiasli) Dec 11, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

could the new event bring benefit to CLI if exit code = 0 could represent successful execution?

I couldn't really get your question. Could you elaborate?

should SUCCESSFUL_EXECUTE be raised after cmd_result is returned?

No. If self.output.out failed, this is considered a failure and SUCCESSFUL_EXECUTE shouldn't be raised by design.

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