Skip to content

Conversation

@nicholasyang2022
Copy link
Collaborator

#1471 adds 3rd level subcommands to crmsh. However, the help module is hardcoded to uses 2 levels of subcommands. This pull request modifies the help module to support arbitrary levels of subcommands.

It consists of 2 parts:

  1. Replace _, the subcommand level separators used in crm.8.adoc, with ., to avoid ambiguity with _ used in subcommand names (such as geo_init).
  2. Use tree structure to store help data in memory, so that it works with arbitrary levels of subcommands.

@nicholasyang2022 nicholasyang2022 force-pushed the multilevel-help branch 2 times, most recently from 8463bc4 to c85362e Compare September 4, 2024 09:24
@codecov
Copy link

codecov bot commented Sep 4, 2024

Codecov Report

Attention: Patch coverage is 80.97561% with 39 lines in your changes missing coverage. Please review.

Project coverage is 69.76%. Comparing base (9aed74c) to head (2d2b9e9).
Report is 13 commits behind head on master.

Files with missing lines Patch % Lines
crmsh/help.py 85.98% 22 Missing ⚠️
crmsh/utils.py 20.00% 16 Missing ⚠️
crmsh/command.py 95.45% 1 Missing ⚠️
Additional details and impacted files
Flag Coverage Δ
integration 54.62% <80.97%> (+0.03%) ⬆️
unit 52.42% <30.73%> (+0.05%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
crmsh/sh.py 92.82% <100.00%> (+0.03%) ⬆️
crmsh/ui_root.py 83.33% <100.00%> (ø)
crmsh/command.py 62.50% <95.45%> (+2.89%) ⬆️
crmsh/utils.py 67.76% <20.00%> (-0.51%) ⬇️
crmsh/help.py 88.62% <85.98%> (+2.34%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@nicholasyang2022 nicholasyang2022 marked this pull request as ready for review September 4, 2024 11:33
crmsh/help.py Outdated
self._cmd_args = cmd_args

@functools.cached_property
def long(self):
Copy link
Collaborator

Choose a reason for hiding this comment

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

rename as long_help might look more descriptive

crmsh/help.py Outdated
args = ['crm']
args.extend(self._cmd_args)
args.append('--help-without-redirect')
rc, stdout = ShellUtils.get_stdout(args, shell=False)
Copy link
Collaborator

Choose a reason for hiding this comment

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

use get_stdout_stderr should be better for storing error msg

return str(self)


class LazyHelpEntryFromCli(HelpEntry):
Copy link
Collaborator

Choose a reason for hiding this comment

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

doc string needed

crmsh/help.py Outdated
children: dict[str, typing.Self]

def is_leaf(self) -> bool:
return bool(self.children)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Typically, a leaf node is a node with, or without children?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I removed this method as it is not used. In recursions, it is more straightforward to loop over children directly without checking whether it is a leaf node.



@dataclasses.dataclass
class SubcommandTreeNode:
Copy link
Collaborator

Choose a reason for hiding this comment

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

docstring needed for this new class

crmsh/help.py Outdated
return max_width


def _render_commmad_tree(out: io.StringIO, node: SubcommandTreeNode, indent: int):
Copy link
Collaborator

Choose a reason for hiding this comment

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

typo here commmad

Copy link
Collaborator

@liangxin1300 liangxin1300 left a comment

Choose a reason for hiding this comment

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

Great work! Thanks!

@liangxin1300 liangxin1300 merged commit 6f44c70 into ClusterLabs:master Sep 6, 2024
@nicholasyang2022 nicholasyang2022 deleted the multilevel-help branch September 9, 2024 07:24
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.

2 participants