_help.py now parses yaml files and updates Helpfile objects #7581

adewaleo · 2018-10-16T21:52:17Z

_help.py parses help.yaml files and updates help objects appropriately
dummy cdn yaml file added
added temporary yaml parser command

Todo:

need to add some sort of tests

This checklist is used to make sure that common guidelines for a pull request are followed.

The PR has modified HISTORY.rst describing any customer-facing, functional changes. Note that this does not include changes only to help content. (see Modifying change log).
I adhere to the Command Guidelines.

tjprescott

First thing: removal the azure-cli-parser module. You can tuck your test command into azure-cli-configure or something, or just rely on az cdn profile create -h.

src/azure-cli-core/azure/cli/core/_help.py

adewaleo · 2018-10-17T23:34:08Z

@tjprescott The parser module has been removed!

williexu · 2018-10-17T23:40:15Z

src/azure-cli-core/azure/cli/core/_help.py

If we're returning on the first iteration, why do we need the loop?

This is because name_source is an array of the option names the parameter can take. E.g. --name and -n.

I don't follow. Right now, you are only looking at the first index of the array.
Is this what you mean to do?

Edited:
Also this code affects how parameters are read from help.py as well. One, can now use just foo or f in help.py, just as in help.yaml, instead of --foo -f

However, I can change the logic to accept only the longer option name, as this might be clearer than accepting either option names.

cc @tjprescott

Thanks for clarifying @williexu, I did not intend to look at just the first element of the array.

@williexu. I made and pushed the change after we spoke

src/azure-cli-core/azure/cli/core/_help.py

tjprescott · 2018-10-18T22:08:41Z

src/azure-cli-core/azure/cli/core/_help.py

+        text = ""
+        for link in links:
+            if "name" in link and "url" in link:
+                text += "- {}: {}.\n".format(link["name"], link["url"])


Name should likely be renamed title since that's the attribute in the HTML tag that corresponds to the tooltip (which is presumably what docs team would use it for). Also, it makes clear that this would be a longer thing than a simple name.

My opinion is that rather than pull this out into a semi-bulleted list, we do:
For more information, visit: <url> and the title wouldn't be used.

tjprescott · 2018-10-18T22:40:39Z

src/azure-cli-core/azure/cli/core/_help.py

+                    value_source.append(link_text.strip())
+            params["populator-commands"] = value_source
+
+        return params


Right now, it looks like this is just copying the new fields into the old fields and relying on the logic that parses the old fields. How much more difficult would it be to actually have this conditional logic in the places that parse the YAML dictionary into the Help objects? This seems like it would be limiting in the long run.

williexu · 2018-10-22T20:07:18Z

src/azure-cli-core/azure/cli/core/_help.py

+            for name in param.name_source:
+                if data.get("name") == name.lstrip("-"):
+                    return True
+            return False


This is a breaking change from the previous behavior- if data.get('name') was "-f --force", this would return false

So, I just tested this scenario and it works. If the data was obtained from _help.py, then HelpFile.load() is called which in turn calls CommandHelpFile._load_from_data() which ensures the previous behavior. These methods are defined in knack.help.py

The change to the previous behaviour is that _help.py will now also allow f or force as names.

src/azure-cli-core/azure/cli/core/_help.py

tjprescott

Like we discussed:

_help.py should contain all of the help classes
_help_util should be renamed _help_loaders and contain just the different loading logic for V0 and V1.
The core logic needs to load V0 first (if present) and then load V1. It is not one or the other.

Parser can output contents as yaml updated help.yaml test file with some comments.

More helpfile updates.

… is empty

…odule.

… Command and Group information formatted differently. Addressed a few other PR comments.

…here some command group names / parameter names are not updated from code.

…ies. Fields within commands/groups need to have some non-alphabetic ordering

…ly appealing. '>' and '|' (part of yaml syntax) are preserved in values.

…puts arguments before examples

create temp directory to hold temporary files and scripts. Updated value-source to value-sources. get_all_help uses CliGroupHelpfile instead of knack's CliGroupHelpfile

adewaleo · 2018-11-26T18:28:05Z

@tjprescott @williexu. Please let me know your thoughts on the current state of the PR. I would still like to add some unittests.

tjprescott · 2018-12-13T16:48:43Z

@adewaleo I'm closing this PR for now since we are essentially doing our reviews in person rather than via GitHub anyways.

adewaleo requested review from tjprescott and williexu October 16, 2018 21:52

tjprescott suggested changes Oct 17, 2018

View reviewed changes

src/azure-cli-core/azure/cli/core/_help.py Outdated Show resolved Hide resolved

williexu reviewed Oct 17, 2018

View reviewed changes

src/azure-cli-core/azure/cli/core/_help.py Outdated Show resolved Hide resolved

williexu reviewed Oct 17, 2018

View reviewed changes

src/azure-cli-core/azure/cli/core/_help.py Outdated Show resolved Hide resolved

williexu reviewed Oct 17, 2018

View reviewed changes

src/azure-cli-core/azure/cli/core/_help.py Outdated Show resolved Hide resolved

williexu reviewed Oct 17, 2018

View reviewed changes

src/azure-cli-core/azure/cli/core/_help.py Outdated Show resolved Hide resolved

adewaleo force-pushed the tosin/Help1 branch from b085cbe to 68234e0 Compare October 17, 2018 23:52

adewaleo requested a review from yugangw-msft October 17, 2018 23:52

adewaleo force-pushed the tosin/Help1 branch from 0d21eb8 to 68234e0 Compare October 18, 2018 00:16

tjprescott suggested changes Oct 18, 2018

View reviewed changes

williexu suggested changes Oct 22, 2018

View reviewed changes

42B reviewed Oct 28, 2018

View reviewed changes

src/azure-cli-core/azure/cli/core/_help.py Outdated Show resolved Hide resolved

src/azure-cli-core/azure/cli/core/_help.py Outdated Show resolved Hide resolved

tjprescott suggested changes Nov 7, 2018

View reviewed changes

Oluwatosin Adewale added 14 commits November 21, 2018 15:48

Dummy help.yaml file

4de2e7d

Parse command for testing yaml parsing

79b5663

Parser can output contents as yaml updated help.yaml test file with some comments.

Added logic to parse help.yaml file and update help data objects.

4226773

More helpfile updates.

_help.py now raises error if it fails to parse help.yaml or help.yaml…

0b73f6c

… is empty

updated _help.py: CLI now prints extension messages. Removed parser m…

04a4901

…odule.

_help.py: Addressed pep8 warnings. Made some methods static

5c95622

changed dir to dir_name as dir is a builtin function.

01ebec5

Added some comments. Link urls have title field instad of name field.…

86e93bb

… Command and Group information formatted differently. Addressed a few other PR comments.

Fixed bug in _params_equal()

857b004

checked out devs _help.py file.

0894682

Refactoring code. Todo: update print_header to print links. Fix bug w…

4a90f31

…here some command group names / parameter names are not updated from code.

script to convert _help.py to help/yaml. Parses short and long summar…

457f6ed

…ies. Fields within commands/groups need to have some non-alphabetic ordering

Help conversion script uses ruamel.yaml. Output is ordered and visual…

43a036f

…ly appealing. '>' and '|' (part of yaml syntax) are preserved in values.

Conversion script now converts parameters to arguments (bug fix). Out…

9e60de9

…puts arguments before examples

Oluwatosin Adewale added 12 commits November 21, 2018 15:52

Removed unnecessary file.

49e6694

Addressed style feedback.

2d8c474

Changes:

47e56d6

create temp directory to hold temporary files and scripts. Updated value-source to value-sources. get_all_help uses CliGroupHelpfile instead of knack's CliGroupHelpfile

uncommented help import

8548e73

Added tests to help script.

bc3a170

PEP8 style changes.

6d5b7f2

Sphinx properly handles help groups.

01ebbf4

Refactored _help.py and _help_util.py to _help_loaders.py

c567248

Removed _help_util.py

8ab7c9c

Bug fixes

9589f39

More refactoring, script tests pass

b5c5279

Changes to restore _help.py to same state before rebase.

a1732d1

adewaleo force-pushed the tosin/Help1 branch from 7469520 to a1732d1 Compare November 22, 2018 00:02

Updated AzCliHelp to print links associated with commands and groups.

dc1ad57

adewaleo changed the base branch from feature/HelpUpdate to dev November 26, 2018 18:21

adewaleo changed the base branch from dev to feature/HelpUpdate November 26, 2018 18:21

tjprescott closed this Dec 13, 2018

_help.py now parses yaml files and updates Helpfile objects #7581

_help.py now parses yaml files and updates Helpfile objects #7581

Uh oh!

Conversation

adewaleo commented Oct 16, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tjprescott left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

adewaleo commented Oct 17, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adewaleo Oct 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adewaleo Oct 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adewaleo Oct 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

tjprescott left a comment

Choose a reason for hiding this comment

Uh oh!

adewaleo commented Nov 26, 2018

Uh oh!

tjprescott commented Dec 13, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

adewaleo commented Oct 16, 2018 •

edited

Loading

adewaleo Oct 22, 2018 •

edited

Loading

adewaleo Oct 22, 2018 •

edited

Loading

adewaleo Oct 22, 2018 •

edited

Loading