Tag glossary #59
Tag glossary #59
Conversation
Added a cli option `-L` to list all tags. * verified `black .` * verified `flake8 .` * verified `mypy .` Bumped the version up via `poetry version patch`
Added a cli option `-L` to list all tags. * verified `black .` * verified `flake8 .` * verified `mypy .` Bumped the version up via `poetry version patch`
|
@dgou hey! I haven't taken a look at the refactor yet, but I wrote this was wondering if I could a review. Appreciate it! Thanks! |
| taglist_filename = output_path / f"{tag_list_name}.rst" | ||
| verbose(f"Writing sphinx tag list: {taglist_filename}") | ||
| taglist_dox.write_to_file(taglist_filename) | ||
|
|
There was a problem hiding this comment.
This code block looks like it was copied from the glossary block right above it.
Being that the previous block has 'return' statements in it, the code might never get to this block.
And if another block is added after this one, a return here might prevent the code from getting to that.
This means restructuring the two blocks. My initial feeling is that this function is already too big :-) but that is a pre-existing condition for this change. I think in the interests of simplicity, I'm willing to leave this as is unless you want to undertake refactoring the glossary and this new tags into helper functions.
There was a problem hiding this comment.
Not at this moment @dgou 😆
| def add_tag(sphinx_dox: SphinxWriter, tag: str) -> SphinxWriter: | ||
| """Map function over tag list.""" | ||
| sphinx_dox.add_output(f"* {tag}") | ||
| sphinx_dox.blank_line() |
There was a problem hiding this comment.
Not sure why the extra blank line (it's more apparent in the test code that the unnumbered list has the extra line.
Not sure if we don't have a helper for that, I need to look at this more in depth.
There was a problem hiding this comment.
This seems to be the pattern in other places where we build sphinx documents.
There was a problem hiding this comment.
Hmm, ok then, you can just use the line_breaks keyword parameter instead, for example: https://github.com/jolly-good-toolbelt/sphinx_gherkindoc/blob/master/sphinx_gherkindoc/writer.py#L105
sphinx_gherkindoc/writer.py
Outdated
| @@ -244,6 +245,8 @@ def ticket_url_or_tag(tag: str) -> str: | |||
| return _value_with_url(tag, url) if url else tag | |||
|
|
|||
| def tags(tags: List[str], *parent_objs: behave.model_core.BasicStatement) -> None: | |||
| # this appends the tags to the previous set | |||
| tag_set.update(set(tags)) | |||
There was a problem hiding this comment.
The set.update method can take any iterable, so you don't need to convert tags to a set first.
casting an interable to `set` is unnecessary
Added functionality for a complete tag list