add new option to enhance first page link #809
Conversation
| public function __construct( | ||
| private readonly Processor $processor, | ||
| private readonly string $pageName = 'page', | ||
| private readonly bool $skipFirstPageLink = false, |
There was a problem hiding this comment.
I'm not so fond of the naming of this parameter as it can be misinterpreted.
| private readonly bool $skipFirstPageLink = false, | |
| private readonly bool $includePageInFIrstPageLink = true, |
| @@ -55,6 +55,9 @@ public function getConfigTreeBuilder(): TreeBuilder | |||
| ->booleanNode('convert_exception') | |||
| ->defaultFalse() | |||
| ->end() | |||
| ->booleanNode('remove_page_1_link') | |||
There was a problem hiding this comment.
I find this configuration option not so straightforward, what bout this? I'm afraid it can be misinterpreted
| ->booleanNode('remove_page_1_link') | |
| ->booleanNode('include_page_in_first_page_link') |
There was a problem hiding this comment.
I understand your concern, I'm open to suggestions to improve the naming. The purpose is explained in the linked issue: to be able to avoid a duplicate URL in pagination, because the first page link is usually an URL like example.org?page=1 and is the same as example.org
There was a problem hiding this comment.
I might have misunderstood the issue but isn't the issue asking only to have the ability to configure a behavior to remove the first page's link page query param? I don't see any duplicated link suppression in the current PR or do you mean duplicated in search engines?
Maybe a more appropriate name would be include_page_param_on_first_page?
There was a problem hiding this comment.
You're right, the duplication refers to the SEO. I'd like to keep the option name under a decent limit, what about "remove_first_page_param"?
Fix #804