Skip to content

[tool] longest_common_prefix - #469

Merged
abduznik merged 3 commits into
abduznik:mainfrom
1998LJ:tool/longest-common-prefix
Sep 18, 2026
Merged

abduznik merged 3 commits into
abduznik:mainfrom
1998LJ:tool/longest-common-prefix

Conversation

@1998LJ

@1998LJ 1998LJ commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Closes #415

Implementation

  • Added tools/longest_common_prefix.py to extract longest common prefix of two comma-separated strings.
  • Tested matching substrings, empty prefixes, and exact matches.

@abduznik abduznik left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Logic is correct but the tool only supports exactly 2 comma-separated strings — the name longest_common_prefix implies it should work on an arbitrary list of strings. Please generalize to accept any number of strings, or clarify in the description that it's 2-string only.

@abduznik

Copy link
Copy Markdown
Owner

Thanks for the contribution! Two quick items before merge:

  1. Missing tests — CONTRIBUTING.md requires tests. Please add a following the existing test pattern.
  2. Header format — The header comment uses 'returns' instead of the arrow format. It should be:
    # longest_common_prefix("flower,flow,flight") → "fl"

Let me know once updated!

@1998LJ

1998LJ commented Sep 17, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the review!

I have updated the PR with the requested changes:

  • Corrected header comment to arrow format (# longest_common_prefix("flower,flow,flight") → "fl").
  • Added multi-string support for comma-separated inputs.
  • Added unit tests in tests/test_longest_common_prefix.py.
  • Synced with upstream main; full test suite passes locally (180 passed).

@abduznik abduznik left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Review comment was stale — code already handles N strings. Merging.

@abduznik
abduznik merged commit 3b27c20 into abduznik:main Sep 18, 2026
2 checks passed
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.

[tool] longest_common_prefix

2 participants