| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
That could make things easier, but I've not used subcommands much in argparse, how are they? We'd need to make sure all the commands and options work in the same way, blurb is used in a few different bits of automation and we wouldn't want to break any of them. |
Sorry, something went wrong.
From my personal usage of subcommands, it works the same. Actually, a subcommand is just an action that invokes an ArgumentParser. However, it might indeed come with some corner cases which I would first extensively test. If you can tell me where it's being used in automaton parts, then I'd be glad to check if I can make the transition. |
Sorry, something went wrong.
Hmm, well at least here: |
Sorry, something went wrong.
There was a problem hiding this comment.
Some initial comments. I'll confer further with Hugo.
Sorry, something went wrong.
|
Thank you Larry for your comments. I'll address them tomorrow (I don't have access to the project now) |
Sorry, something went wrong.
- remove section IDs matching - do not render the table in case of a multi-match - simplify `add` docstring construction - update tests - update README.md
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
I ended up making them match like that (the tests should reflect what is now possible). I don't have a better way to cover the use cases so don't hesistate to tell me which cases should be checked. |
Sorry, something went wrong.
|
Would it be sufficient to create a "nickname map", like { "".join(section.split()).lower(): section for section in sections } |
Sorry, something went wrong.
Yes. But to construct additional patterns, I needed to use: _sanitized = re.sub(r'[ /]', ' ', _section)
_section_words = re.split(r'\s+', _sanitized)
_section_names_lower_nosep[_section] = ''.join(_section_words).lower()(Your suggestion is in my case implemented as ''.join(_section_words).lower()) |
Sorry, something went wrong.
There was a problem hiding this comment.
I'm not fully sold on the smart matching, I don't think we need to worry about things like blurb add --section "cOre _ and - bUILtins".
Would case-insensitive substring matching be enough, as long as the substring is unique?
So these would work:
blurb add --section mac blurb add --section core blurb add --section build blurb add --section built
But not:
blurb add --section buil
blurb add --section i
Sorry, something went wrong.
| if issue.startswith('gh-'): | ||
| issue = issue[3:] |
There was a problem hiding this comment.
| if issue.startswith('gh-'): | |
| issue = issue[3:] | |
| issue = issue.removeprefix('gh-') |
Sorry, something went wrong.
| del _sanitized | ||
| # '_', '-', ' ' and '/' are the allowed (user) separators | ||
| _section_pattern = r'[_\- /]?'.join(map(re.escape, _section_words)) | ||
| # add '$' to avoid matching after the pattern | ||
| _section_pattern = f'{_section_pattern}$' | ||
| del _section_words | ||
| _section_pattern = re.compile(_section_pattern, re.I) | ||
| _section_special_patterns[_section].add(_section_pattern) | ||
| del _section_pattern, _section |
There was a problem hiding this comment.
Do we really need these dels?
| del _sanitized | |
| # '_', '-', ' ' and '/' are the allowed (user) separators | |
| _section_pattern = r'[_\- /]?'.join(map(re.escape, _section_words)) | |
| # add '$' to avoid matching after the pattern | |
| _section_pattern = f'{_section_pattern}$' | |
| del _section_words | |
| _section_pattern = re.compile(_section_pattern, re.I) | |
| _section_special_patterns[_section].add(_section_pattern) | |
| del _section_pattern, _section | |
| # '_', '-', ' ' and '/' are the allowed (user) separators | |
| _section_pattern = r'[_\- /]?'.join(map(re.escape, _section_words)) | |
| # add '$' to avoid matching after the pattern | |
| _section_pattern = f'{_section_pattern}$' | |
| _section_pattern = re.compile(_section_pattern, re.I) | |
| _section_special_patterns[_section].add(_section_pattern) |
Sorry, something went wrong.
| del _sec_name_width | ||
| sections_table = '\n'.join(map(_format_row, sections)) | ||
| del _format_row | ||
| sections_table = '\n'.join((_sec_row_rule, sections_table, _sec_row_rule)) | ||
| del _sec_row_rule |
There was a problem hiding this comment.
| del _sec_name_width | |
| sections_table = '\n'.join(map(_format_row, sections)) | |
| del _format_row | |
| sections_table = '\n'.join((_sec_row_rule, sections_table, _sec_row_rule)) | |
| del _sec_row_rule | |
| sections_table = '\n'.join(map(_format_row, sections)) | |
| sections_table = '\n'.join((_sec_row_rule, sections_table, _sec_row_rule)) |
Sorry, something went wrong.
| def check(section, expect): | ||
| actual = blurb._extract_section_name(section) | ||
| assert actual == expect |
There was a problem hiding this comment.
To match the other test files:
| def check(section, expect): | |
| actual = blurb._extract_section_name(section) | |
| assert actual == expect | |
| def check(section, expected): | |
| actual = blurb._extract_section_name(section) | |
| assert actual == expected |
Sorry, something went wrong.
| res = blurb._update_blurb_template(issue=None, section=section) | ||
| res = res.splitlines() | ||
| for section_name in blurb.sections: | ||
| if section_name == expect: |
There was a problem hiding this comment.
| if section_name == expect: | |
| if section_name == expected: |
Sorry, something went wrong.
| ('section', 'expect'), | ||
| tuple(zip(blurb.sections, blurb.sections)) | ||
| ) | ||
| def test_exact_names(self, section, expect): | ||
| self.check(section, expect) | ||
|
|
||
| @pytest.mark.parametrize( | ||
| ('section', 'expect'), [ |
There was a problem hiding this comment.
| ('section', 'expect'), | |
| tuple(zip(blurb.sections, blurb.sections)) | |
| ) | |
| def test_exact_names(self, section, expect): | |
| self.check(section, expect) | |
| @pytest.mark.parametrize( | |
| ('section', 'expect'), [ | |
| ('section', 'expected'), | |
| tuple(zip(blurb.sections, blurb.sections)) | |
| ) | |
| def test_exact_names(self, section, expected): | |
| self.check(section, expected) | |
| @pytest.mark.parametrize( | |
| ('section', 'expected'), [ |
Sorry, something went wrong.
| def test_partial_words(self, section, expect): | ||
| self.check(section, expect) | ||
|
|
||
| @pytest.mark.parametrize( | ||
| ('section', 'expect'), [ |
There was a problem hiding this comment.
| def test_partial_words(self, section, expect): | |
| self.check(section, expect) | |
| @pytest.mark.parametrize( | |
| ('section', 'expect'), [ | |
| def test_partial_words(self, section, expected): | |
| self.check(section, expected) | |
| @pytest.mark.parametrize( | |
| ('section', 'expected'), [ |
Sorry, something went wrong.
| def test_partial_special_names(self, section, expect): | ||
| self.check(section, expect) | ||
|
|
||
| @pytest.mark.parametrize( | ||
| ('section', 'expect'), [ |
There was a problem hiding this comment.
| def test_partial_special_names(self, section, expect): | |
| self.check(section, expect) | |
| @pytest.mark.parametrize( | |
| ('section', 'expect'), [ | |
| def test_partial_special_names(self, section, expected): | |
| self.check(section, expected) | |
| @pytest.mark.parametrize( | |
| ('section', 'expected'), [ |
Sorry, something went wrong.
| def test_partial_separators(self, section, expect): | ||
| # normalize the separtors '_', '-', ' ' and '/' | ||
| self.check(section, expect) | ||
|
|
||
| @pytest.mark.parametrize( | ||
| ('prefix', 'expect'), [ |
There was a problem hiding this comment.
Plus a typo
| def test_partial_separators(self, section, expect): | |
| # normalize the separtors '_', '-', ' ' and '/' | |
| self.check(section, expect) | |
| @pytest.mark.parametrize( | |
| ('prefix', 'expect'), [ | |
| def test_partial_separators(self, section, expected): | |
| # normalize the separators '_', '-', ' ' and '/' | |
| self.check(section, expected) | |
| @pytest.mark.parametrize( | |
| ('prefix', 'expected'), [ |
Sorry, something went wrong.
| def test_partial_prefix_words(self, prefix, expect): | ||
| # try to find a match using prefixes (without separators and lowercase) | ||
| self.check(prefix, expect) | ||
|
|
||
| @pytest.mark.parametrize( | ||
| ('section', 'expect'), | ||
| [(name.lower(), name) for name in blurb.sections], | ||
| ) | ||
| def test_exact_names_lowercase(self, section, expect): | ||
| self.check(section, expect) | ||
|
|
||
| @pytest.mark.parametrize( | ||
| ('section', 'expect'), | ||
| [(name.upper(), name) for name in blurb.sections], | ||
| ) | ||
| def test_exact_names_uppercase(self, section, expect): | ||
| self.check(section, expect) |
There was a problem hiding this comment.
| def test_partial_prefix_words(self, prefix, expect): | |
| # try to find a match using prefixes (without separators and lowercase) | |
| self.check(prefix, expect) | |
| @pytest.mark.parametrize( | |
| ('section', 'expect'), | |
| [(name.lower(), name) for name in blurb.sections], | |
| ) | |
| def test_exact_names_lowercase(self, section, expect): | |
| self.check(section, expect) | |
| @pytest.mark.parametrize( | |
| ('section', 'expect'), | |
| [(name.upper(), name) for name in blurb.sections], | |
| ) | |
| def test_exact_names_uppercase(self, section, expect): | |
| self.check(section, expect) | |
| def test_partial_prefix_words(self, prefix, expected): | |
| # try to find a match using prefixes (without separators and lowercase) | |
| self.check(prefix, expected) | |
| @pytest.mark.parametrize( | |
| ('section', 'expected'), | |
| [(name.lower(), name) for name in blurb.sections], | |
| ) | |
| def test_exact_names_lowercase(self, section, expected): | |
| self.check(section, expected) | |
| @pytest.mark.parametrize( | |
| ('section', 'expected'), | |
| [(name.upper(), name) for name in blurb.sections], | |
| ) | |
| def test_exact_names_uppercase(self, section, expected): | |
| self.check(section, expected) |
Sorry, something went wrong.
|
I think it would be enough. |
Sorry, something went wrong.
Integrate the user-friendly features from PR python#16 by @picnixz into the automation support from PR python#45, making the CLI more intuitive: - Change --gh-issue to --issue, accepting multiple formats: * Plain numbers: --issue 12345 * With gh- prefix: --issue gh-12345 * GitHub URLs: --issue python/cpython#12345 - Add smart section matching with: * Case-insensitive matching: --section lib matches "Library" * Partial matching: --section doc matches "Documentation" * Common aliases: --section api matches "C API" * Separator normalization: --section core-and-builtins - Improve error messages for invalid sections This combines the automation features from PR python#45 with the interface improvements suggested by @picnixz in PR python#16, as reviewed by @hugovk and @larryhastings. Co-authored-by: picnixz <picnixz@users.noreply.github.com>
Integrate the user-friendly features from PR python#16 by @picnixz into the automation support from PR python#45, making the CLI more intuitive: - Change --gh-issue to --issue, accepting multiple formats: * Plain numbers: --issue 12345 * With gh- prefix: --issue gh-12345 * GitHub URLs: --issue python/cpython#12345 - Add smart section matching with: * Case-insensitive matching: --section lib matches "Library" * Partial matching: --section doc matches "Documentation" * Common aliases: --section api matches "C API" * Separator normalization: --section core-and-builtins - Improve error messages for invalid sections This combines the automation features from PR python#45 with the interface improvements suggested by @picnixz in PR python#16, as reviewed by @hugovk and @larryhastings.
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Fixes #6. This is an alternative to #15.
@hugovk I've taken out your way of handling positional arguments (but would you consider a PR which refactors blurb in order to use argparse instead?). If you prefer --gh, I can also rename the variable!