[RTM] Use ausi/slug-generator for the creation of aliases #1016

ausi · 2017-08-10T18:16:01Z

As discussed in #954

@contao/developers Is an event class the right thing for this use case?

ToDo:

Wait until CLDR 32.0 is released (19th October) and tag version 1.0.0 of ausi/slug-generator
Create pull requests for news-bundle and events-bundle: [RTM] Use the slug generator service for the news alias news-bundle#20, [RTM] Use the slug generator service for the event alias calendar-bundle#14

Toflar

Yes, an event is perfectly correct. Excellent stuff 👍

agoat · 2017-08-11T18:42:25Z

Have only looked quickly over it. Great.

Just as a note: Your package require php: ^7.1. If this will be merged, contao should also raise the minimum php version to 7.1.

Toflar · 2017-08-11T19:26:47Z

It already is ;) See the develop branch :)

ausi · 2017-08-11T21:27:49Z

Yes, the PHP requirement is the same as in Contao 4.5. But it also requires ext-intl, ext-mbstring and ext-pcre, maybe we should discuss if it is OK to require these extensions?

leofeyer · 2017-08-11T21:43:33Z

I guess so. Is there an alternative?

Toflar · 2017-08-11T22:43:09Z

Of course! All of them are required for a decent multilingual (and thus multibyte/utf-8) setup.

ausi · 2017-08-14T16:08:25Z

ext-pcre is already required by Contao and patchwork/utf8.

ext-intl and ext-mbstring are already required by league/uri which itself is required since Contao 4.1.

Therefore I think I can keep the requirements of the slug library as they are now.

ausi · 2017-10-30T15:43:27Z

The de-ASCII rules are included in the latest CLDR version 32-beta3, so I released ausi/slug-generator version 1.0 and updated this PR.

It should be RTM now 🎉

leofeyer · 2017-10-31T14:21:14Z

src/Slug/ValidCharacters.php

+    /**
+     * @var string[]
+     */
+    private $defaultOptions = [


Could this also be a constant or static property?

Yes, a private constant should be fine.

ausi · 2017-10-31T14:21:14Z

tests/Slug/ValidCharactersTest.php


+        $translator
+            ->expects($this->at(1))


Is it necessary to test how the translator is used?

Probably not. 😄

Although the test assures that the class iterates over all four default options.

But the order should not be relevant. I think we should remove the translator tests here.

ausi · 2017-10-31T14:22:40Z

src/Event/ContaoCoreEvents.php

@@ -60,7 +60,7 @@
    public const BACKEND_MENU_BUILD = 'contao.backend_menu_build';

    /**
-     * The contao.slug_valid_characters event is triggered when the options for the valid characters setting for aliases are generated.
+     * The contao.slug_valid_characters event is triggered when the valid slug characters option is generated.


I think this should be plural “options are generated”

Are there several valid slug characters options?

Yes, by default there are four options: unicode lowercase, unicode, ASCII lowercase and ASCII.

ausi · 2017-10-31T14:23:16Z

src/Resources/contao/languages/en/tl_page.xlf

      </trans-unit>
      <trans-unit id="tl_page.validAliasCharacters.1">
-        <source>When aliases get created automatically, this character set gets used for the conversion.</source>
+        <source>Please choose the character for auto-generating page aliases.</source>


Also plural “characters”

Description ----------- See contao/core-bundle#1016 Commits ------- ac1a2f9 Use the slug generator service for the event alias d9b5537 Handle the $objCalendar === null case 30bf980 Fix a wrong nesting

Description ----------- See contao/core-bundle#1016 Commits ------- 2e286b0 Use the slug generator service for the news alias fb5b802 Also handle the $objNewsArchive === null case

…dle#1016).

leofeyer · 2017-10-31T17:20:31Z

Very good job @ausi. Thanks a bunch.

backbone87 · 2018-01-21T18:51:36Z

So 4.4 LTS does not and will not support this?

ausi · 2018-01-21T20:32:01Z

@backbone87 No, but there is an extension which backports the new behaviour to 4.4: ausi/contao-slug-backport

backbone87 · 2018-01-21T21:47:32Z

@ausi is it worth to add a contao specific impl of Ausi\SlugGenerator\SlugGenerator that provides a method with a PageModel argument, that automatically extracts the slug configuration and builds it? that would remove the duplicated code that gathers this configuration in various listeners and eases the integration for 3rd party bundles

ausi · 2018-01-22T09:57:32Z

I’m not sure if this is necessary. It’s not that much code that transforms the page model to the options array:

$slugOptions['locale'] = $objPage->language;
if ($objPage->validAliasCharacters) {
    $slugOptions['validChars'] = $objPage->validAliasCharacters;
}

backbone87 · 2018-01-22T10:36:53Z

well, you also must make sure, that $page->loadDetails() is executed and possibly strip insert tags.

further StringUtil::generateAlias should be deprecated and changed to use the slug generator

ausi · 2018-01-22T12:06:01Z

further StringUtil::generateAlias should be deprecated and changed to use the slug generator

sounds reasonable, I think you should open a new ticket for that.

ausi · 2018-01-22T18:47:09Z

Related: #1334

Description ----------- Fixes #798 I am not sure about the performance implications here as the `file_exists()` check is done inside the `while` loop. We could also do ```php if (!include $strParent) { throw new \Exception('…'); } ``` @contao/developers WDYT? Commits ------- 91e1c928 Check if a template exists when inheriting templates (see #798) 10c057e8 Only log the missing file instead of throwing an exception

Toflar approved these changes Aug 11, 2017

View reviewed changes

leofeyer assigned ausi Aug 11, 2017

leofeyer added the feature label Aug 11, 2017

leofeyer added this to the 4.5.0 milestone Aug 11, 2017

leofeyer mentioned this pull request Aug 15, 2017

[WIP] Add the contao.slugify service #569

Closed

3 tasks

ausi changed the title ~~[WIP] Use ausi/slug-generator for the creation of aliases~~ [RFC] Use ausi/slug-generator for the creation of aliases Sep 12, 2017

This was referenced Sep 13, 2017

[RTM] Use the slug generator service for the news alias contao/news-bundle#20

Merged

[RTM] Use the slug generator service for the event alias contao/calendar-bundle#14

Merged

Alias-Umschreibung ä zu ae mit setLocale funktioniert nicht mehr #954

Closed

leofeyer force-pushed the develop branch 5 times, most recently from ed0a7e5 to 3dadd44 Compare September 28, 2017 10:39

leofeyer force-pushed the develop branch from e37e989 to dad087d Compare October 3, 2017 19:03

This was referenced Oct 6, 2017

4.4.x - Navigation: Umlaute werden nicht sauber umschrieben #1132

Closed

Re #1132 - Umlaute im Alias #1134

Closed

leofeyer force-pushed the develop branch from f131963 to e665dae Compare October 17, 2017 07:50

ausi force-pushed the feature/slug-generator branch from b22e057 to 868e9e6 Compare October 30, 2017 15:21

ausi changed the title ~~[RFC] Use ausi/slug-generator for the creation of aliases~~ [RTM] Use ausi/slug-generator for the creation of aliases Oct 30, 2017

ausi added 3 commits October 31, 2017 14:37

Use ausi/slug-generator for the creation of aliases

e38a33a

Use version 1.0.0@dev

8e7ca00

Rename valid characters option name

0d4c33f

ausi and others added 4 commits October 31, 2017 14:37

Test SlugValidCharactersEvent and Slug\ValidCharacters

2cd1d54

Use stable version of ausi/slug-generator

7e52491

Use the PHP 7 syntax.

6fc8732

Use the translator to generate the labels.

a04c20f

leofeyer force-pushed the feature/slug-generator branch from 868e9e6 to a04c20f Compare October 31, 2017 14:15

leofeyer reviewed Oct 31, 2017

View reviewed changes

ausi commented Oct 31, 2017

View reviewed changes

Implement the adjustments as discussed.

fe8286a

leofeyer force-pushed the feature/slug-generator branch from 401d88e to fe8286a Compare October 31, 2017 14:43

Add a change log entry.

63b7996

leofeyer merged commit a075de1 into contao:develop Oct 31, 2017

leofeyer added a commit to contao/faq-bundle that referenced this pull request Oct 31, 2017

Use ausi/slug-generator to auto-generate aliases (see contao/core-bun…

2aba9d8

…dle#1016).

leofeyer added a commit to contao/newsletter-bundle that referenced this pull request Oct 31, 2017

Use ausi/slug-generator to auto-generate aliases (see contao/core-bun…

459e998

…dle#1016).

leofeyer mentioned this pull request Oct 31, 2017

News or Event -Datail : Alias with "Umlaute" don´t work contao/core#8802

Closed

ausi mentioned this pull request Jan 22, 2018

auto generated alias converts some special characters to numbers #1334

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RTM] Use ausi/slug-generator for the creation of aliases #1016

[RTM] Use ausi/slug-generator for the creation of aliases #1016

ausi commented Aug 10, 2017 •

edited

Loading

Toflar left a comment

agoat commented Aug 11, 2017

Toflar commented Aug 11, 2017 via email

ausi commented Aug 11, 2017

leofeyer commented Aug 11, 2017

Toflar commented Aug 11, 2017 via email

ausi commented Aug 14, 2017

ausi commented Oct 30, 2017

leofeyer Oct 31, 2017

ausi Oct 31, 2017

ausi Oct 31, 2017 •

edited

Loading

leofeyer Oct 31, 2017

leofeyer Oct 31, 2017

ausi Oct 31, 2017

ausi Oct 31, 2017

leofeyer Oct 31, 2017

ausi Oct 31, 2017

ausi Oct 31, 2017

leofeyer Oct 31, 2017

leofeyer commented Oct 31, 2017

backbone87 commented Jan 21, 2018

ausi commented Jan 21, 2018

backbone87 commented Jan 21, 2018

ausi commented Jan 22, 2018

backbone87 commented Jan 22, 2018

ausi commented Jan 22, 2018

ausi commented Jan 22, 2018

[RTM] Use ausi/slug-generator for the creation of aliases #1016

[RTM] Use ausi/slug-generator for the creation of aliases #1016

Conversation

ausi commented Aug 10, 2017 • edited Loading

Toflar left a comment

Choose a reason for hiding this comment

agoat commented Aug 11, 2017

Toflar commented Aug 11, 2017 via email

ausi commented Aug 11, 2017

leofeyer commented Aug 11, 2017

Toflar commented Aug 11, 2017 via email

ausi commented Aug 14, 2017

ausi commented Oct 30, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ausi Oct 31, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leofeyer commented Oct 31, 2017

backbone87 commented Jan 21, 2018

ausi commented Jan 21, 2018

backbone87 commented Jan 21, 2018

ausi commented Jan 22, 2018

backbone87 commented Jan 22, 2018

ausi commented Jan 22, 2018

ausi commented Jan 22, 2018

ausi commented Aug 10, 2017 •

edited

Loading

ausi Oct 31, 2017 •

edited

Loading