[BUGFIX] Improve URL generation for empty filenames and known documents #1282
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
CybotTM
changed the title
Empty filename with anchor should return '#anchor' (same-page link), not '.html#anchor' which is invalid. Use case: referencing an anchor on the current page without specifying the document name.
- Handle empty string anchor same as null (no trailing #) - Remove unused getCurrentFileName mock from test - Add edge case tests for empty string anchor - Rename test case for clarity (empty anchor → null anchor)
- Create InvalidUrlException for better error semantics - Fix known document handling (remove /prefix hack, use directly) - Add clarifying comment for email validation (mailto: support)
URLs to pages without anchors should not have a trailing #
jaapio
force-pushed
the
fix/empty-filename-url-generator
branch
from
8b01df0 to
4e8a6a1
Compare
Member
|
Thanks this looks good to me. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Improves URL generation in
AbstractUrlGeneratorwith fixes for empty filenames, known document handling, and code quality improvements.Changes
1. Empty Filename Support (bugfix)
When
filenameis empty (same-page anchor reference):.html#anchoror.html(invalid URLs)#anchoror#(correct same-page links)2. Known Document Handling (bugfix)
Fixed the TODO hack for known documents:
/then passed throughcanonicalUrl()- a workaround#from URLs without anchors (e.g.,/page1.htmlinstead of/page1.html#)3. Code Quality Improvements (refactor)
InvalidUrlExceptionfor better error semantics (was genericException)mailto:link support)InvalidUrlExceptionon absolute URLsFiles Changed
Source:
packages/guides/src/Renderer/UrlGenerator/AbstractUrlGenerator.phppackages/guides/src/Renderer/UrlGenerator/Exception/InvalidUrlException.php(new)Tests:
packages/guides/tests/unit/Renderer/UrlGenerator/RelativeUrlGeneratorTest.phptests/Integration/tests/toctree/toctree-external-linktargets/expected/index.htmltests/Integration/tests-full/bootstrap/bootstrap-menu-external-linktargets/expected/index.htmltests/Integration/tests-full/bootstrap/bootstrap-menu-external-on-subpage/expected/index.htmlTest Plan