Skip to content

Optimize task list detection in li()#108

Closed
BenjaminHoegh wants to merge 1 commit intodevfrom
codex/improve-performance-of-li-function
Closed

Optimize task list detection in li()#108
BenjaminHoegh wants to merge 1 commit intodevfrom
codex/improve-performance-of-li-function

Conversation

@BenjaminHoegh
Copy link
Copy Markdown
Owner

Summary

  • improve performance of li by eliminating regex
  • handle paragraph removal and interruption checks more efficiently

Testing

  • vendor/bin/phpunit --version (fails: command not found)

https://chatgpt.com/codex/tasks/task_e_686bd62da0f48321b338519e54b40c48

Copilot AI review requested due to automatic review settings July 7, 2025 14:31
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

A refactor of the li() method to replace regex-based task list detection with manual character checks and to streamline paragraph interruption and tag removal.

  • Replaced preg_match logic with manual substring checks for task checkbox markers.
  • Introduced an $interrupted flag for blank-line interruption handling.
  • Enhanced paragraph tag stripping to remove both opening and closing <p> correctly.
Comments suppressed due to low confidence (1)

src/ParsedownExtended.php:2185

  • Consider adding unit tests to cover the new task list detection logic in both legacy and non-legacy branches, including cases with checked and unchecked items, and paragraph interruption scenarios.
        $interrupted = in_array('', $lines, true);

Comment on lines +2190 to 2205
$firstFour = substr($markup, 4, 4);
$checked = false;
if (
isset($firstFour[0], $firstFour[1], $firstFour[2], $firstFour[3]) &&
$firstFour[0] === '[' &&
$firstFour[2] === ']' &&
($firstFour[1] === ' ' || $firstFour[1] === 'x' || $firstFour[1] === 'X')
) {
$checked = ($firstFour[1] === 'x' || $firstFour[1] === 'X');
$replace = '<input type="checkbox" disabled="disabled"';
if ($checked) {
$replace .= ' checked="checked"';
}
$replace .= ' />';
$markup = substr_replace($markup, $replace, 4, 4);
}
Copy link

Copilot AI Jul 7, 2025

Choose a reason for hiding this comment

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

[nitpick] There's duplicated manual task list detection logic in both the legacy and non-legacy branches – consider extracting this into a helper method to reduce code duplication and improve readability.

Suggested change
$firstFour = substr($markup, 4, 4);
$checked = false;
if (
isset($firstFour[0], $firstFour[1], $firstFour[2], $firstFour[3]) &&
$firstFour[0] === '[' &&
$firstFour[2] === ']' &&
($firstFour[1] === ' ' || $firstFour[1] === 'x' || $firstFour[1] === 'X')
) {
$checked = ($firstFour[1] === 'x' || $firstFour[1] === 'X');
$replace = '<input type="checkbox" disabled="disabled"';
if ($checked) {
$replace .= ' checked="checked"';
}
$replace .= ' />';
$markup = substr_replace($markup, $replace, 4, 4);
}
$markup = $this->processTaskListItem($markup);

Copilot uses AI. Check for mistakes.
Comment on lines +2222 to +2223
$text =& $Elements[0]['handler']['argument'];
$firstFour = substr($text, 0, 4);
Copy link

Copilot AI Jul 7, 2025

Choose a reason for hiding this comment

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

[nitpick] Using the reference operator & when assigning $text can be confusing; consider modifying $Elements[0]['handler']['argument'] directly instead of using a variable reference.

Suggested change
$text =& $Elements[0]['handler']['argument'];
$firstFour = substr($text, 0, 4);
$firstFour = substr($Elements[0]['handler']['argument'], 0, 4);

Copilot uses AI. Check for mistakes.
@BenjaminHoegh BenjaminHoegh deleted the codex/improve-performance-of-li-function branch July 8, 2025 05:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants