Skip to content
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.

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

https://github.com/jackdewinter/pymarkdown/issues/818 - Initial work #1129

Merged
merged 11 commits into from
Jul 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
25 changes: 18 additions & 7 deletions newdocs/src/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,24 @@

### Fixed

- [None](https://github.com/jackdewinter/pymarkdown/issues/1120)
- https://github.com/jackdewinter/pymarkdown/issues/1122
- https://github.com/jackdewinter/pymarkdown/issues/1123
- https://github.com/jackdewinter/pymarkdown/issues/1124
https://github.com/jackdewinter/pymarkdown/issues/1125
https://github.com/jackdewinter/pymarkdown/issues/1126
https://github.com/jackdewinter/pymarkdown/issues/1127
- [Issue 1120](https://github.com/jackdewinter/pymarkdown/issues/1120)
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion (documentation): Add a colon after 'Issue 1120'.

For consistency, add a colon after 'Issue 1120'.

Suggested change
- [Issue 1120](https://github.com/jackdewinter/pymarkdown/issues/1120)
[Issue 1120](https://github.com/jackdewinter/pymarkdown/issues/1120):

- within Block-List, thematic break can sometimes not report newlines to the
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion (documentation): Rephrase for better readability.

Consider rephrasing to 'thematic break sometimes does not report newlines' for better readability.

Suggested change
- within Block-List, thematic break can sometimes not report newlines to the
- within Block-List, thematic break sometimes does not report newlines to the

list block
- [Issue 1122](https://github.com/jackdewinter/pymarkdown/issues/1122)
- opening a fenced code block in a Bq-List-Bq was closing the outer BQ
Copy link
Contributor

Choose a reason for hiding this comment

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

issue (documentation): Inconsistent capitalization of 'BQ'.

Ensure 'BQ' is consistently capitalized as 'Bq' to match the rest of the document.

- [Issue 1123](https://github.com/jackdewinter/pymarkdown/issues/1123)
- in some cases within a Bq-List-Bq, not counting the newlines properly
- [Issue 1124](https://github.com/jackdewinter/pymarkdown/issues/1124)
- list items within a Bq-List-Bq can have incorrect starting text regarding
the innermost block
- [Issue 1125](https://github.com/jackdewinter/pymarkdown/issues/1125)
- parsing of blank lines within Bq-List-Bq does not always add the right
newlines to the list
- [Issue 1126](https://github.com/jackdewinter/pymarkdown/issues/1126)
- under some circumstances, with a Bq-List-Bq, thematic break can cause
the block quote to close
- [Issue 1127](https://github.com/jackdewinter/pymarkdown/issues/1127)
- rehydration can be wrong with indented blocks in Bq-List-Bq
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion (documentation): Rephrase 'rehydration can be wrong'.

Consider rephrasing to 'rehydration can fail' or 'rehydration can be incorrect' for clarity.

Suggested change
- rehydration can be wrong with indented blocks in Bq-List-Bq
- rehydration can be incorrect with indented blocks in Bq-List-Bq


### Changed

Expand Down
8 changes: 4 additions & 4 deletions publish/coverage.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@
"projectName": "pymarkdown",
"reportSource": "pytest",
"branchLevel": {
"totalMeasured": 4941,
"totalCovered": 4941
"totalMeasured": 5023,
"totalCovered": 5023
},
"lineLevel": {
"totalMeasured": 19691,
"totalCovered": 19691
"totalMeasured": 19960,
"totalCovered": 19960
}
}

17 changes: 10 additions & 7 deletions publish/pylint_suppression.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
},
"pymarkdown/application_logging.py": {},
"pymarkdown/block_quotes/block_quote_count_helper.py": {
"too-many-arguments": 7
"too-many-arguments": 8
},
"pymarkdown/block_quotes/block_quote_data.py": {},
"pymarkdown/block_quotes/block_quote_non_fenced_helper.py": {
Expand All @@ -24,7 +24,8 @@
"too-many-locals": 1
},
"pymarkdown/block_quotes/block_quote_processor.py": {
"too-many-arguments": 4
"too-many-arguments": 5,
"too-many-locals": 2
},
"pymarkdown/coalesce/coalesce_processor.py": {
"too-few-public-methods": 1
Expand Down Expand Up @@ -320,7 +321,9 @@
"pymarkdown/plugins/rule_md_030.py": {
"too-many-instance-attributes": 1
},
"pymarkdown/plugins/rule_md_031.py": {},
"pymarkdown/plugins/rule_md_031.py": {
"too-many-instance-attributes": 1
},
"pymarkdown/plugins/rule_md_032.py": {},
"pymarkdown/plugins/rule_md_033.py": {},
"pymarkdown/plugins/rule_md_034.py": {
Expand Down Expand Up @@ -483,7 +486,7 @@
"too-many-boolean-expressions": 1
},
"pymarkdown/transform_markdown/transform_list_block.py": {
"too-many-arguments": 2
"too-many-arguments": 4
},
"pymarkdown/transform_markdown/transform_new_list_item.py": {
"too-few-public-methods": 1,
Expand All @@ -496,11 +499,11 @@
"pymarkdown/version.py": {}
},
"disables-by-name": {
"too-many-instance-attributes": 24,
"too-many-instance-attributes": 25,
"too-many-public-methods": 4,
"too-few-public-methods": 39,
"too-many-arguments": 227,
"too-many-locals": 40,
"too-many-arguments": 231,
"too-many-locals": 42,
"chained-comparison": 1,
"too-many-boolean-expressions": 2,
"protected-access": 25,
Expand Down
10 changes: 5 additions & 5 deletions publish/test-results.json
Original file line number Diff line number Diff line change
Expand Up @@ -1300,7 +1300,7 @@
},
{
"name": "test.rules.test_md027",
"totalTests": 109,
"totalTests": 110,
"failedTests": 0,
"errorTests": 0,
"skippedTests": 0,
Expand Down Expand Up @@ -1364,7 +1364,7 @@
},
{
"name": "test.rules.test_md031",
"totalTests": 25,
"totalTests": 110,
"failedTests": 0,
"errorTests": 0,
"skippedTests": 0,
Expand Down Expand Up @@ -1620,10 +1620,10 @@
},
{
"name": "test.test_markdown_extra",
"totalTests": 148,
"totalTests": 163,
"failedTests": 0,
"errorTests": 0,
"skippedTests": 0,
"skippedTests": 1,
"elapsedTimeInMilliseconds": 0
},
{
Expand Down Expand Up @@ -1692,7 +1692,7 @@
},
{
"name": "test.tokens.test_markdown_token",
"totalTests": 3,
"totalTests": 5,
"failedTests": 0,
"errorTests": 0,
"skippedTests": 0,
Expand Down
165 changes: 120 additions & 45 deletions pymarkdown/block_quotes/block_quote_count_helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@
ParagraphStackToken,
)

# pylint: disable=too-many-lines


POGGER = ParserLogger(logging.getLogger(__name__))


Expand Down Expand Up @@ -149,6 +152,41 @@ def __handle_bq_whitespace(adjusted_line: str, start_index: int) -> Tuple[str, i
start_index += 1
return adjusted_line, start_index

# pylint: disable=too-many-arguments
@staticmethod
def __xx(
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: Rename method __xx to a more descriptive name.

The method name __xx is not descriptive. Consider renaming it to something that reflects its functionality.

Suggested change
def __xx(
def parse_block_quote(

parser_state: ParserState,
adjusted_line: str,
start_index: int,
current_count: int,
stack_count: int,
last_block_quote_index: int,
) -> Tuple[bool, int, int, int]:
(
continue_processing,
start_index,
) = BlockQuoteCountHelper.__is_special_double_block_case(
parser_state, adjusted_line, start_index, current_count, stack_count
)
if not continue_processing and current_count < stack_count:
continue_proc, stack_token_index = BlockQuoteCountHelper.__xx_part_one(
parser_state, start_index, current_count
)
if continue_proc:
current_count, start_index, last_block_quote_index = (
BlockQuoteCountHelper.__xx_part_two(
parser_state,
stack_token_index,
start_index,
current_count,
stack_count,
last_block_quote_index,
)
)
return continue_processing, current_count, start_index, last_block_quote_index

# pylint: enable=too-many-arguments

# pylint: disable=too-many-arguments
@staticmethod
def __should_continue_processing(
Expand Down Expand Up @@ -214,14 +252,17 @@ def __should_continue_processing(
):
(
continue_processing,
current_count,
start_index,
last_block_quote_index,
) = BlockQuoteCountHelper.__xx(
parser_state,
adjusted_line,
start_index,
) = BlockQuoteCountHelper.__is_special_double_block_case(
parser_state, adjusted_line, start_index, current_count, stack_count
current_count,
stack_count,
last_block_quote_index,
)
if not continue_processing and current_count < stack_count:
continue_proc, stack_token_index = BlockQuoteCountHelper.__xx_part_one(parser_state, start_index, current_count, stack_count)
if continue_proc:
current_count, start_index, last_block_quote_index = BlockQuoteCountHelper.__xx_part_two(parser_state, stack_token_index, start_index, current_count, stack_count, last_block_quote_index)
else:
continue_processing = True
return (
Expand All @@ -234,47 +275,77 @@ def __should_continue_processing(
)

@staticmethod
def __xx_part_one(parser_state:ParserState, start_index, current_count, stack_count):
def __xx_part_one(
parser_state: ParserState, start_index: int, current_count: int
) -> Tuple[bool, int]:
if parser_state.token_stack[-1].is_fenced_code_block:
return False, -1
block_quote_character_count = ParserHelper.count_characters_in_text(parser_state.original_line_to_parse[:start_index], ">")
assert parser_state.original_line_to_parse is not None
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: Consider handling the case where original_line_to_parse is None.

While the assert statement ensures original_line_to_parse is not None, it might be more robust to handle this case explicitly, perhaps by raising a custom exception.

Suggested change
assert parser_state.original_line_to_parse is not None
if parser_state.original_line_to_parse is None:
raise ValueError("original_line_to_parse cannot be None")

block_quote_character_count = ParserHelper.count_characters_in_text(
parser_state.original_line_to_parse[:start_index], ">"
)
if block_quote_character_count > current_count:
return False, -1
count_block_quotes = 0
for stack_token_index in range(len(parser_state.token_stack)):
if parser_state.token_stack[stack_token_index].is_block_quote:
found_index = -2
for stack_token_index, stack_token in enumerate(
parser_state.token_stack
): # pragma: no cover
if stack_token.is_block_quote:
count_block_quotes += 1
if count_block_quotes == block_quote_character_count:
found_index = stack_token_index
break
assert stack_token_index != len(parser_state.token_stack), "should have completed before this"
stack_token_index += 1
return not parser_state.token_stack[stack_token_index].is_block_quote, stack_token_index
assert found_index != -2
assert found_index != len(
parser_state.token_stack
), "should have completed before this"
found_index += 1
return (
not parser_state.token_stack[found_index].is_block_quote,
found_index,
)

@staticmethod
def __xx_part_two(parser_state:ParserState, stack_index, start_index, current_count, stack_count, last_block_quote_index):
def __xx_part_two(
parser_state: ParserState,
stack_index: int,
start_index: int,
current_count: int,
stack_count: int,
last_block_quote_index: int,
) -> Tuple[int, int, int]:
# At this point, we have a "prefix", which may be partial, that has the
# current_count of > characters, and ends with a list. If we are here,
# we know that previous lines have had at least one more > character and
# counted block quote.
assert parser_state.token_stack[stack_index].is_list, "If not bq, must be a list."
assert parser_state.token_stack[
stack_index
].is_list, "If not bq, must be a list."
while parser_state.token_stack[stack_index].is_list:
stack_index += 1
embedded_list_stack_token = parser_state.token_stack[stack_index-1]
if parser_state.original_line_to_parse[start_index:embedded_list_stack_token.indent_level].strip():
embedded_list_stack_token = cast(
ListStackToken, parser_state.token_stack[stack_index - 1]
)
assert parser_state.original_line_to_parse is not None
if parser_state.original_line_to_parse[
start_index : embedded_list_stack_token.indent_level
].strip():
return current_count, start_index, last_block_quote_index
assert current_count + 1 == stack_count
if (
parser_state.original_line_to_parse[
embedded_list_stack_token.indent_level
]
parser_state.original_line_to_parse[embedded_list_stack_token.indent_level]
!= ">"
):
return current_count, start_index, last_block_quote_index
last_block_quote_index = embedded_list_stack_token.indent_level + 1
if last_block_quote_index < len(parser_state.original_line_to_parse):
character_after_block_quote = parser_state.original_line_to_parse[last_block_quote_index]
if character_after_block_quote == " ":
last_block_quote_index += 1
character_after_block_quote = parser_state.original_line_to_parse[
last_block_quote_index
]
assert character_after_block_quote == " "
# if character_after_block_quote == " ":
last_block_quote_index += 1

return current_count + 1, last_block_quote_index, last_block_quote_index

Expand Down Expand Up @@ -389,13 +460,14 @@ def __increase_stack(
stack_count,
block_quote_data,
)
if not skip:
block_quote_data = BlockQuoteCountHelper.decrease_stack_to_level(
parser_state,
block_quote_data.current_count,
stack_count,
container_level_tokens,
)
assert not skip
# if not skip:
block_quote_data = BlockQuoteCountHelper.decrease_stack_to_level(
parser_state,
block_quote_data.current_count,
stack_count,
container_level_tokens,
)
POGGER.debug(
"container_level_tokens>>$",
container_level_tokens,
Expand Down Expand Up @@ -955,21 +1027,24 @@ def __calculate_eligible_stack_hard_limit(
indent_text_count += delta
length_of_available_whitespace -= delta
extra_consumed_whitespace += delta
if adjust_current_block_quote:
POGGER.debug(
"__calculate_stack_hard_limit>>last_block_token>>$",
parser_state.token_stack[last_bq_index].matching_markdown_token,
)
block_token = cast(
BlockQuoteMarkdownToken,
parser_state.token_stack[last_bq_index].matching_markdown_token,
)
block_token.add_bleading_spaces(
ParserHelper.repeat_string(ParserHelper.space_character, delta), True
)
POGGER.debug(
"__calculate_stack_hard_limit>>last_block_token>>$", block_token
)

assert not adjust_current_block_quote
_ = last_bq_index
# if adjust_current_block_quote:
# POGGER.debug(
# "__calculate_stack_hard_limit>>last_block_token>>$",
# parser_state.token_stack[last_bq_index].matching_markdown_token,
# )
# block_token = cast(
# BlockQuoteMarkdownToken,
# parser_state.token_stack[last_bq_index].matching_markdown_token,
# )
# block_token.add_bleading_spaces(
# ParserHelper.repeat_string(ParserHelper.space_character, delta), True
# )
# POGGER.debug(
# "__calculate_stack_hard_limit>>last_block_token>>$", block_token
# )

return (
current_stack_index,
Expand Down
Loading
Loading