Skip to content

Conversation

donquixote
Copy link
Contributor

The values passed to MyersDiff::calculate() don't need to be strings.
All we need is a comparison function that accepts the value types.

Comment on lines 78 to 82
* @param list<array{int, int}> $snakes Common subsequences
* @param list<T> $a First sequence
* @param list<T> $b Second sequence
*
* @return list<array{T, -1,0,1}> - pairs of token and edit (-1 for delete, 0 for keep, +1 for insert)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For the record:
I am used to Drupal doc style which I find more readable:

Suggested change
* @param list<array{int, int}> $snakes Common subsequences
* @param list<T> $a First sequence
* @param list<T> $b Second sequence
*
* @return list<array{T, -1,0,1}> - pairs of token and edit (-1 for delete, 0 for keep, +1 for insert)
* @param list<array{int, int}> $snakes
* Common subsequences.
* @param list<T> $a
* First sequence.
* @param list<T> $b
* Second sequence.
*
* @return list<array{T, -1|0|1}>
* Pairs of token and edit (-1 for delete, 0 for keep, +1 for insert)

@fisharebest fisharebest merged commit 22c3044 into fisharebest:main Apr 26, 2025
12 of 13 checks passed
@fisharebest
Copy link
Owner

Thanks for this. PHPDOC syntax has moved on since I wrote this package.

@donquixote
Copy link
Contributor Author

Thanks for the quick merging!

Thanks for this. PHPDOC syntax has moved on since I wrote this package.

The @template stuff is mostly for tools like phpstan and psalm, but IDEs also support it, at least partially.

@fisharebest
Copy link
Owner

Looking again at this, I think the $a and $b parameters to calculate() should be arrays after all (not lists).

In one my applications that uses this library, the keys are not consecutive integers, which is why the function calles $a = array_values($a)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants