From: ArthurHoaro Date: Thu, 22 Oct 2020 14:21:03 +0000 (+0200) Subject: Feature: support any tag separator X-Git-Tag: v0.12.1^2~7^2~3 X-Git-Url: https://git.immae.eu/?p=github%2Fshaarli%2FShaarli.git;a=commitdiff_plain;h=b3bd8c3e8d367975980043e772f7cd78b7f96bc6 Feature: support any tag separator So it allows to have multiple words tags. Breaking change: commas ',' are no longer a default separator. Fixes #594 --- diff --git a/application/bookmark/Bookmark.php b/application/bookmark/Bookmark.php index 4810c5e6..8aaeb9d8 100644 --- a/application/bookmark/Bookmark.php +++ b/application/bookmark/Bookmark.php @@ -60,11 +60,13 @@ class Bookmark /** * Initialize a link from array data. Especially useful to create a Bookmark from former link storage format. * - * @param array $data + * @param array $data + * @param string $tagsSeparator Tags separator loaded from the config file. + * This is a context data, and it should *never* be stored in the Bookmark object. * * @return $this */ - public function fromArray(array $data): Bookmark + public function fromArray(array $data, string $tagsSeparator = ' '): Bookmark { $this->id = $data['id'] ?? null; $this->shortUrl = $data['shorturl'] ?? null; @@ -77,7 +79,7 @@ class Bookmark if (is_array($data['tags'])) { $this->tags = $data['tags']; } else { - $this->tags = preg_split('/\s+/', $data['tags'] ?? '', -1, PREG_SPLIT_NO_EMPTY); + $this->tags = tags_str2array($data['tags'] ?? '', $tagsSeparator); } if (! empty($data['updated'])) { $this->updated = $data['updated']; @@ -348,7 +350,12 @@ class Bookmark */ public function setTags(?array $tags): Bookmark { - $this->setTagsString(implode(' ', $tags ?? [])); + $this->tags = array_map( + function (string $tag): string { + return $tag[0] === '-' ? substr($tag, 1) : $tag; + }, + tags_filter($tags, ' ') + ); return $this; } @@ -420,11 +427,13 @@ class Bookmark } /** - * @return string Bookmark's tags as a string, separated by a space + * @param string $separator Tags separator loaded from the config file. + * + * @return string Bookmark's tags as a string, separated by a separator */ - public function getTagsString(): string + public function getTagsString(string $separator = ' '): string { - return implode(' ', $this->getTags()); + return tags_array2str($this->getTags(), $separator); } /** @@ -444,19 +453,13 @@ class Bookmark * - trailing dash in tags will be removed * * @param string|null $tags + * @param string $separator Tags separator loaded from the config file. * * @return $this */ - public function setTagsString(?string $tags): Bookmark + public function setTagsString(?string $tags, string $separator = ' '): Bookmark { - // Remove first '-' char in tags. - $tags = preg_replace('/(^| )\-/', '$1', $tags ?? ''); - // Explode all tags separted by spaces or commas - $tags = preg_split('/[\s,]+/', $tags); - // Remove eventual empty values - $tags = array_values(array_filter($tags)); - - $this->tags = $tags; + $this->setTags(tags_str2array($tags, $separator)); return $this; } @@ -507,7 +510,7 @@ class Bookmark */ public function renameTag(string $fromTag, string $toTag): void { - if (($pos = array_search($fromTag, $this->tags)) !== false) { + if (($pos = array_search($fromTag, $this->tags ?? [])) !== false) { $this->tags[$pos] = trim($toTag); } } @@ -519,7 +522,7 @@ class Bookmark */ public function deleteTag(string $tag): void { - if (($pos = array_search($tag, $this->tags)) !== false) { + if (($pos = array_search($tag, $this->tags ?? [])) !== false) { unset($this->tags[$pos]); $this->tags = array_values($this->tags); } diff --git a/application/bookmark/BookmarkFileService.php b/application/bookmark/BookmarkFileService.php index 3ea98a45..85efeea6 100644 --- a/application/bookmark/BookmarkFileService.php +++ b/application/bookmark/BookmarkFileService.php @@ -91,7 +91,7 @@ class BookmarkFileService implements BookmarkServiceInterface } } - $this->bookmarkFilter = new BookmarkFilter($this->bookmarks); + $this->bookmarkFilter = new BookmarkFilter($this->bookmarks, $this->conf); } /** diff --git a/application/bookmark/BookmarkFilter.php b/application/bookmark/BookmarkFilter.php index c79386ea..5d8733dc 100644 --- a/application/bookmark/BookmarkFilter.php +++ b/application/bookmark/BookmarkFilter.php @@ -6,6 +6,7 @@ namespace Shaarli\Bookmark; use Exception; use Shaarli\Bookmark\Exception\BookmarkNotFoundException; +use Shaarli\Config\ConfigManager; /** * Class LinkFilter. @@ -58,12 +59,16 @@ class BookmarkFilter */ private $bookmarks; + /** @var ConfigManager */ + protected $conf; + /** * @param Bookmark[] $bookmarks initialization. */ - public function __construct($bookmarks) + public function __construct($bookmarks, ConfigManager $conf) { $this->bookmarks = $bookmarks; + $this->conf = $conf; } /** @@ -107,10 +112,14 @@ class BookmarkFilter $filtered = $this->bookmarks; } if (!empty($request[0])) { - $filtered = (new BookmarkFilter($filtered))->filterTags($request[0], $casesensitive, $visibility); + $filtered = (new BookmarkFilter($filtered, $this->conf)) + ->filterTags($request[0], $casesensitive, $visibility) + ; } if (!empty($request[1])) { - $filtered = (new BookmarkFilter($filtered))->filterFulltext($request[1], $visibility); + $filtered = (new BookmarkFilter($filtered, $this->conf)) + ->filterFulltext($request[1], $visibility) + ; } return $filtered; case self::$FILTER_TEXT: @@ -280,8 +289,9 @@ class BookmarkFilter * * @return string generated regex fragment */ - private static function tag2regex(string $tag): string + protected function tag2regex(string $tag): string { + $tagsSeparator = $this->conf->get('general.tags_separator', ' '); $len = strlen($tag); if (!$len || $tag === "-" || $tag === "*") { // nothing to search, return empty regex @@ -295,12 +305,13 @@ class BookmarkFilter $i = 0; // start at first character $regex = '(?='; // use positive lookahead } - $regex .= '.*(?:^| )'; // before tag may only be a space or the beginning + // before tag may only be the separator or the beginning + $regex .= '.*(?:^|' . $tagsSeparator . ')'; // iterate over string, separating it into placeholder and content for (; $i < $len; $i++) { if ($tag[$i] === '*') { // placeholder found - $regex .= '[^ ]*?'; + $regex .= '[^' . $tagsSeparator . ']*?'; } else { // regular characters $offset = strpos($tag, '*', $i); @@ -316,7 +327,8 @@ class BookmarkFilter $i = $offset; } } - $regex .= '(?:$| ))'; // after the tag may only be a space or the end + // after the tag may only be the separator or the end + $regex .= '(?:$|' . $tagsSeparator . '))'; return $regex; } @@ -334,14 +346,15 @@ class BookmarkFilter */ public function filterTags($tags, bool $casesensitive = false, string $visibility = 'all') { + $tagsSeparator = $this->conf->get('general.tags_separator', ' '); // get single tags (we may get passed an array, even though the docs say different) $inputTags = $tags; if (!is_array($tags)) { // we got an input string, split tags - $inputTags = preg_split('/(?:\s+)|,/', $inputTags, -1, PREG_SPLIT_NO_EMPTY); + $inputTags = tags_str2array($inputTags, $tagsSeparator); } - if (!count($inputTags)) { + if (count($inputTags) === 0) { // no input tags return $this->noFilter($visibility); } @@ -358,7 +371,7 @@ class BookmarkFilter } // build regex from all tags - $re = '/^' . implode(array_map("self::tag2regex", $inputTags)) . '.*$/'; + $re = '/^' . implode(array_map([$this, 'tag2regex'], $inputTags)) . '.*$/'; if (!$casesensitive) { // make regex case insensitive $re .= 'i'; @@ -378,7 +391,8 @@ class BookmarkFilter continue; } } - $search = $link->getTagsString(); // build search string, start with tags of current link + // build search string, start with tags of current link + $search = $link->getTagsString($tagsSeparator); if (strlen(trim($link->getDescription())) && strpos($link->getDescription(), '#') !== false) { // description given and at least one possible tag found $descTags = array(); @@ -390,9 +404,9 @@ class BookmarkFilter ); if (count($descTags[1])) { // there were some tags in the description, add them to the search string - $search .= ' ' . implode(' ', $descTags[1]); + $search .= $tagsSeparator . tags_array2str($descTags[1], $tagsSeparator); } - }; + } // match regular expression with search string if (!preg_match($re, $search)) { // this entry does _not_ match our regex @@ -422,7 +436,7 @@ class BookmarkFilter } } - if (empty(trim($link->getTagsString()))) { + if (empty($link->getTags())) { $filtered[$key] = $link; } } @@ -537,10 +551,11 @@ class BookmarkFilter */ protected function buildFullTextSearchableLink(Bookmark $link, array &$lengths): string { + $tagString = $link->getTagsString($this->conf->get('general.tags_separator', ' ')); $content = mb_convert_case($link->getTitle(), MB_CASE_LOWER, 'UTF-8') .'\\'; $content .= mb_convert_case($link->getDescription(), MB_CASE_LOWER, 'UTF-8') .'\\'; $content .= mb_convert_case($link->getUrl(), MB_CASE_LOWER, 'UTF-8') .'\\'; - $content .= mb_convert_case($link->getTagsString(), MB_CASE_LOWER, 'UTF-8') .'\\'; + $content .= mb_convert_case($tagString, MB_CASE_LOWER, 'UTF-8') .'\\'; $lengths['title'] = ['start' => 0, 'end' => mb_strlen($link->getTitle())]; $nextField = $lengths['title']['end'] + 1; @@ -548,7 +563,7 @@ class BookmarkFilter $nextField = $lengths['description']['end'] + 1; $lengths['url'] = ['start' => $nextField, 'end' => $nextField + mb_strlen($link->getUrl())]; $nextField = $lengths['url']['end'] + 1; - $lengths['tags'] = ['start' => $nextField, 'end' => $nextField + mb_strlen($link->getTagsString())]; + $lengths['tags'] = ['start' => $nextField, 'end' => $nextField + mb_strlen($tagString)]; return $content; } diff --git a/application/bookmark/LinkUtils.php b/application/bookmark/LinkUtils.php index 17c37979..9493b0aa 100644 --- a/application/bookmark/LinkUtils.php +++ b/application/bookmark/LinkUtils.php @@ -176,3 +176,49 @@ function is_note($linkUrl) { return isset($linkUrl[0]) && $linkUrl[0] === '?'; } + +/** + * Extract an array of tags from a given tag string, with provided separator. + * + * @param string|null $tags String containing a list of tags separated by $separator. + * @param string $separator Shaarli's default: ' ' (whitespace) + * + * @return array List of tags + */ +function tags_str2array(?string $tags, string $separator): array +{ + // For whitespaces, we use the special \s regex character + $separator = $separator === ' ' ? '\s' : $separator; + + return preg_split('/\s*' . $separator . '+\s*/', trim($tags) ?? '', -1, PREG_SPLIT_NO_EMPTY); +} + +/** + * Return a tag string with provided separator from a list of tags. + * Note that given array is clean up by tags_filter(). + * + * @param array|null $tags List of tags + * @param string $separator + * + * @return string + */ +function tags_array2str(?array $tags, string $separator): string +{ + return implode($separator, tags_filter($tags, $separator)); +} + +/** + * Clean an array of tags: trim + remove empty entries + * + * @param array|null $tags List of tags + * @param string $separator + * + * @return array + */ +function tags_filter(?array $tags, string $separator): array +{ + $trimDefault = " \t\n\r\0\x0B"; + return array_values(array_filter(array_map(function (string $entry) use ($separator, $trimDefault): string { + return trim($entry, $trimDefault . $separator); + }, $tags ?? []))); +} diff --git a/application/config/ConfigManager.php b/application/config/ConfigManager.php index fb085023..a035baae 100644 --- a/application/config/ConfigManager.php +++ b/application/config/ConfigManager.php @@ -368,6 +368,7 @@ class ConfigManager $this->setEmpty('general.default_note_title', 'Note: '); $this->setEmpty('general.retrieve_description', true); $this->setEmpty('general.enable_async_metadata', true); + $this->setEmpty('general.tags_separator', ' '); $this->setEmpty('updates.check_updates', false); $this->setEmpty('updates.check_updates_branch', 'stable'); diff --git a/application/formatter/BookmarkDefaultFormatter.php b/application/formatter/BookmarkDefaultFormatter.php index 149a3eb9..51bea0f1 100644 --- a/application/formatter/BookmarkDefaultFormatter.php +++ b/application/formatter/BookmarkDefaultFormatter.php @@ -68,15 +68,16 @@ class BookmarkDefaultFormatter extends BookmarkFormatter */ protected function formatTagListHtml($bookmark) { + $tagsSeparator = $this->conf->get('general.tags_separator', ' '); if (empty($bookmark->getAdditionalContentEntry('search_highlight')['tags'])) { return $this->formatTagList($bookmark); } $tags = $this->tokenizeSearchHighlightField( - $bookmark->getTagsString(), + $bookmark->getTagsString($tagsSeparator), $bookmark->getAdditionalContentEntry('search_highlight')['tags'] ); - $tags = $this->filterTagList(explode(' ', $tags)); + $tags = $this->filterTagList(tags_str2array($tags, $tagsSeparator)); $tags = escape($tags); $tags = $this->replaceTokensArray($tags); @@ -88,7 +89,7 @@ class BookmarkDefaultFormatter extends BookmarkFormatter */ protected function formatTagString($bookmark) { - return implode(' ', $this->formatTagList($bookmark)); + return implode($this->conf->get('general.tags_separator'), $this->formatTagList($bookmark)); } /** diff --git a/application/formatter/BookmarkFormatter.php b/application/formatter/BookmarkFormatter.php index e1b7f705..124ce78b 100644 --- a/application/formatter/BookmarkFormatter.php +++ b/application/formatter/BookmarkFormatter.php @@ -267,7 +267,7 @@ abstract class BookmarkFormatter */ protected function formatTagString($bookmark) { - return implode(' ', $this->formatTagList($bookmark)); + return implode($this->conf->get('general.tags_separator', ' '), $this->formatTagList($bookmark)); } /** @@ -351,6 +351,7 @@ abstract class BookmarkFormatter /** * Format tag list, e.g. remove private tags if the user is not logged in. + * TODO: this method is called multiple time to format tags, the result should be cached. * * @param array $tags * diff --git a/application/front/controller/admin/ManageTagController.php b/application/front/controller/admin/ManageTagController.php index 2065c3e2..22fb461c 100644 --- a/application/front/controller/admin/ManageTagController.php +++ b/application/front/controller/admin/ManageTagController.php @@ -24,6 +24,12 @@ class ManageTagController extends ShaarliAdminController $fromTag = $request->getParam('fromtag') ?? ''; $this->assignView('fromtag', escape($fromTag)); + $separator = escape($this->container->conf->get('general.tags_separator', ' ')); + if ($separator === ' ') { + $separator = ' '; + $this->assignView('tags_separator_desc', t('whitespace')); + } + $this->assignView('tags_separator', $separator); $this->assignView( 'pagetitle', t('Manage tags') .' - '. $this->container->conf->get('general.title', 'Shaarli') @@ -85,4 +91,31 @@ class ManageTagController extends ShaarliAdminController return $this->redirect($response, $redirect); } + + /** + * POST /admin/tags/change-separator - Change tag separator + */ + public function changeSeparator(Request $request, Response $response): Response + { + $this->checkToken($request); + + $reservedCharacters = ['-', '.', '*']; + $newSeparator = $request->getParam('separator'); + if ($newSeparator === null || mb_strlen($newSeparator) !== 1) { + $this->saveErrorMessage(t('Tags separator must be a single character.')); + } elseif (in_array($newSeparator, $reservedCharacters, true)) { + $reservedCharacters = implode(' ', array_map(function (string $character) { + return '' . $character . ''; + }, $reservedCharacters)); + $this->saveErrorMessage( + t('These characters are reserved and can\'t be used as tags separator: ') . $reservedCharacters + ); + } else { + $this->container->conf->set('general.tags_separator', $newSeparator, true, true); + + $this->saveSuccessMessage('Your tags separator setting has been updated!'); + } + + return $this->redirect($response, '/admin/tags'); + } } diff --git a/application/front/controller/admin/ShaareManageController.php b/application/front/controller/admin/ShaareManageController.php index 2ed298f5..0b143172 100644 --- a/application/front/controller/admin/ShaareManageController.php +++ b/application/front/controller/admin/ShaareManageController.php @@ -125,7 +125,7 @@ class ShaareManageController extends ShaarliAdminController // To preserve backward compatibility with 3rd parties, plugins still use arrays $data = $formatter->format($bookmark); $this->executePageHooks('save_link', $data); - $bookmark->fromArray($data); + $bookmark->fromArray($data, $this->container->conf->get('general.tags_separator', ' ')); $this->container->bookmarkService->set($bookmark, false); ++$count; @@ -167,7 +167,7 @@ class ShaareManageController extends ShaarliAdminController // To preserve backward compatibility with 3rd parties, plugins still use arrays $data = $formatter->format($bookmark); $this->executePageHooks('save_link', $data); - $bookmark->fromArray($data); + $bookmark->fromArray($data, $this->container->conf->get('general.tags_separator', ' ')); $this->container->bookmarkService->set($bookmark); diff --git a/application/front/controller/admin/ShaarePublishController.php b/application/front/controller/admin/ShaarePublishController.php index 18afc2d1..625a5680 100644 --- a/application/front/controller/admin/ShaarePublishController.php +++ b/application/front/controller/admin/ShaarePublishController.php @@ -113,7 +113,10 @@ class ShaarePublishController extends ShaarliAdminController $bookmark->setDescription($request->getParam('lf_description')); $bookmark->setUrl($request->getParam('lf_url'), $this->container->conf->get('security.allowed_protocols', [])); $bookmark->setPrivate(filter_var($request->getParam('lf_private'), FILTER_VALIDATE_BOOLEAN)); - $bookmark->setTagsString($request->getParam('lf_tags')); + $bookmark->setTagsString( + $request->getParam('lf_tags'), + $this->container->conf->get('general.tags_separator', ' ') + ); if ($this->container->conf->get('thumbnails.mode', Thumbnailer::MODE_NONE) !== Thumbnailer::MODE_NONE && true !== $this->container->conf->get('general.enable_async_metadata', true) @@ -128,7 +131,7 @@ class ShaarePublishController extends ShaarliAdminController $data = $formatter->format($bookmark); $this->executePageHooks('save_link', $data); - $bookmark->fromArray($data); + $bookmark->fromArray($data, $this->container->conf->get('general.tags_separator', ' ')); $this->container->bookmarkService->set($bookmark); // If we are called from the bookmarklet, we must close the popup: @@ -221,6 +224,11 @@ class ShaarePublishController extends ShaarliAdminController protected function buildFormData(array $link, bool $isNew, Request $request): array { + $link['tags'] = strlen($link['tags']) > 0 + ? $link['tags'] . $this->container->conf->get('general.tags_separator', ' ') + : $link['tags'] + ; + return escape([ 'link' => $link, 'link_is_new' => $isNew, diff --git a/application/front/controller/visitor/BookmarkListController.php b/application/front/controller/visitor/BookmarkListController.php index 78c474c9..cc3837ce 100644 --- a/application/front/controller/visitor/BookmarkListController.php +++ b/application/front/controller/visitor/BookmarkListController.php @@ -95,6 +95,10 @@ class BookmarkListController extends ShaarliVisitorController $next_page_url = '?page=' . ($page - 1) . $searchtermUrl . $searchtagsUrl; } + $tagsSeparator = $this->container->conf->get('general.tags_separator', ' '); + $searchTagsUrlEncoded = array_map('urlencode', tags_str2array($searchTags, $tagsSeparator)); + $searchTags = !empty($searchTags) ? trim($searchTags, $tagsSeparator) . $tagsSeparator : ''; + // Fill all template fields. $data = array_merge( $this->initializeTemplateVars(), @@ -106,7 +110,7 @@ class BookmarkListController extends ShaarliVisitorController 'result_count' => count($linksToDisplay), 'search_term' => escape($searchTerm), 'search_tags' => escape($searchTags), - 'search_tags_url' => array_map('urlencode', explode(' ', $searchTags)), + 'search_tags_url' => $searchTagsUrlEncoded, 'visibility' => $visibility, 'links' => $linkDisp, ] @@ -119,8 +123,9 @@ class BookmarkListController extends ShaarliVisitorController return '[' . $tag . ']'; }; $data['pagetitle'] .= ! empty($searchTags) - ? implode(' ', array_map($bracketWrap, preg_split('/\s+/', $searchTags))) . ' ' - : ''; + ? implode(' ', array_map($bracketWrap, tags_str2array($searchTags, $tagsSeparator))) . ' ' + : '' + ; $data['pagetitle'] .= '- '; } diff --git a/application/front/controller/visitor/TagCloudController.php b/application/front/controller/visitor/TagCloudController.php index 76ed7690..560cad08 100644 --- a/application/front/controller/visitor/TagCloudController.php +++ b/application/front/controller/visitor/TagCloudController.php @@ -47,13 +47,14 @@ class TagCloudController extends ShaarliVisitorController */ protected function processRequest(string $type, Request $request, Response $response): Response { + $tagsSeparator = $this->container->conf->get('general.tags_separator', ' '); if ($this->container->loginManager->isLoggedIn() === true) { $visibility = $this->container->sessionManager->getSessionParameter('visibility'); } $sort = $request->getQueryParam('sort'); $searchTags = $request->getQueryParam('searchtags'); - $filteringTags = $searchTags !== null ? explode(' ', $searchTags) : []; + $filteringTags = $searchTags !== null ? explode($tagsSeparator, $searchTags) : []; $tags = $this->container->bookmarkService->bookmarksCountPerTag($filteringTags, $visibility ?? null); @@ -71,8 +72,9 @@ class TagCloudController extends ShaarliVisitorController $tagsUrl[escape($tag)] = urlencode((string) $tag); } - $searchTags = implode(' ', escape($filteringTags)); - $searchTagsUrl = urlencode(implode(' ', $filteringTags)); + $searchTags = tags_array2str($filteringTags, $tagsSeparator); + $searchTags = !empty($searchTags) ? trim($searchTags, $tagsSeparator) . $tagsSeparator : ''; + $searchTagsUrl = urlencode($searchTags); $data = [ 'search_tags' => escape($searchTags), 'search_tags_url' => $searchTagsUrl, @@ -82,7 +84,7 @@ class TagCloudController extends ShaarliVisitorController $this->executePageHooks('render_tag' . $type, $data, 'tag.' . $type); $this->assignAllView($data); - $searchTags = !empty($searchTags) ? $searchTags .' - ' : ''; + $searchTags = !empty($searchTags) ? trim(str_replace($tagsSeparator, ' ', $searchTags)) .' - ' : ''; $this->assignView( 'pagetitle', $searchTags . t('Tag '. $type) .' - '. $this->container->conf->get('general.title', 'Shaarli') diff --git a/application/front/controller/visitor/TagController.php b/application/front/controller/visitor/TagController.php index de4e7ea2..7a3377a7 100644 --- a/application/front/controller/visitor/TagController.php +++ b/application/front/controller/visitor/TagController.php @@ -45,9 +45,10 @@ class TagController extends ShaarliVisitorController unset($params['addtag']); } + $tagsSeparator = $this->container->conf->get('general.tags_separator', ' '); // Check if this tag is already in the search query and ignore it if it is. // Each tag is always separated by a space - $currentTags = isset($params['searchtags']) ? explode(' ', $params['searchtags']) : []; + $currentTags = tags_str2array($params['searchtags'] ?? '', $tagsSeparator); $addtag = true; foreach ($currentTags as $value) { @@ -62,7 +63,7 @@ class TagController extends ShaarliVisitorController $currentTags[] = trim($newTag); } - $params['searchtags'] = trim(implode(' ', $currentTags)); + $params['searchtags'] = tags_array2str($currentTags, $tagsSeparator); // We also remove page (keeping the same page has no sense, since the results are different) unset($params['page']); @@ -98,10 +99,11 @@ class TagController extends ShaarliVisitorController } if (isset($params['searchtags'])) { - $tags = explode(' ', $params['searchtags']); + $tagsSeparator = $this->container->conf->get('general.tags_separator', ' '); + $tags = tags_str2array($params['searchtags'] ?? '', $tagsSeparator); // Remove value from array $tags. $tags = array_diff($tags, [$tagToRemove]); - $params['searchtags'] = implode(' ', $tags); + $params['searchtags'] = tags_array2str($tags, $tagsSeparator); if (empty($params['searchtags'])) { unset($params['searchtags']); diff --git a/application/http/HttpAccess.php b/application/http/HttpAccess.php index 646a5264..e80e0c01 100644 --- a/application/http/HttpAccess.php +++ b/application/http/HttpAccess.php @@ -29,14 +29,16 @@ class HttpAccess &$title, &$description, &$keywords, - $retrieveDescription + $retrieveDescription, + $tagsSeparator ) { return get_curl_download_callback( $charset, $title, $description, $keywords, - $retrieveDescription + $retrieveDescription, + $tagsSeparator ); } diff --git a/application/http/HttpUtils.php b/application/http/HttpUtils.php index 28c12969..ed1002b0 100644 --- a/application/http/HttpUtils.php +++ b/application/http/HttpUtils.php @@ -550,7 +550,8 @@ function get_curl_download_callback( &$title, &$description, &$keywords, - $retrieveDescription + $retrieveDescription, + $tagsSeparator ) { $currentChunk = 0; $foundChunk = null; @@ -568,6 +569,7 @@ function get_curl_download_callback( */ return function ($ch, $data) use ( $retrieveDescription, + $tagsSeparator, &$charset, &$title, &$description, @@ -598,10 +600,10 @@ function get_curl_download_callback( if (! empty($keywords)) { $foundChunk = $currentChunk; // Keywords use the format tag1, tag2 multiple words, tag - // So we format them to match Shaarli's separator and glue multiple words with '-' - $keywords = implode(' ', array_map(function($keyword) { - return implode('-', preg_split('/\s+/', trim($keyword))); - }, explode(',', $keywords))); + // So we split the result with `,`, then if a tag contains the separator we replace it by `-`. + $keywords = tags_array2str(array_map(function(string $keyword) use ($tagsSeparator): string { + return tags_array2str(tags_str2array($keyword, $tagsSeparator), '-'); + }, tags_str2array($keywords, ',')), $tagsSeparator); } } diff --git a/application/http/MetadataRetriever.php b/application/http/MetadataRetriever.php index ba9bd40c..2e1401ec 100644 --- a/application/http/MetadataRetriever.php +++ b/application/http/MetadataRetriever.php @@ -38,7 +38,6 @@ class MetadataRetriever $title = null; $description = null; $tags = null; - $retrieveDescription = $this->conf->get('general.retrieve_description'); // Short timeout to keep the application responsive // The callback will fill $charset and $title with data from the downloaded page. @@ -52,7 +51,8 @@ class MetadataRetriever $title, $description, $tags, - $retrieveDescription + $this->conf->get('general.retrieve_description'), + $this->conf->get('general.tags_separator', ' ') ) ); diff --git a/application/legacy/LegacyUpdater.php b/application/legacy/LegacyUpdater.php index fe1a286f..ed949b1e 100644 --- a/application/legacy/LegacyUpdater.php +++ b/application/legacy/LegacyUpdater.php @@ -585,7 +585,7 @@ class LegacyUpdater $linksArray = new BookmarkArray(); foreach ($this->linkDB as $key => $link) { - $linksArray[$key] = (new Bookmark())->fromArray($link); + $linksArray[$key] = (new Bookmark())->fromArray($link, $this->conf->get('general.tags_separator', ' ')); } $linksIo = new BookmarkIO($this->conf); $linksIo->write($linksArray); diff --git a/application/netscape/NetscapeBookmarkUtils.php b/application/netscape/NetscapeBookmarkUtils.php index b83f16f8..6ca728b7 100644 --- a/application/netscape/NetscapeBookmarkUtils.php +++ b/application/netscape/NetscapeBookmarkUtils.php @@ -101,11 +101,11 @@ class NetscapeBookmarkUtils // Add tags to all imported bookmarks? if (empty($post['default_tags'])) { - $defaultTags = array(); + $defaultTags = []; } else { - $defaultTags = preg_split( - '/[\s,]+/', - escape($post['default_tags']) + $defaultTags = tags_str2array( + escape($post['default_tags']), + $this->conf->get('general.tags_separator', ' ') ); } @@ -171,7 +171,7 @@ class NetscapeBookmarkUtils $link->setUrl($bkm['uri'], $this->conf->get('security.allowed_protocols')); $link->setDescription($bkm['note']); $link->setPrivate($private); - $link->setTagsString($bkm['tags']); + $link->setTags($bkm['tags']); $this->bookmarkService->addOrSet($link, false); $importCount++; diff --git a/application/render/PageBuilder.php b/application/render/PageBuilder.php index c2fae705..bf0ae326 100644 --- a/application/render/PageBuilder.php +++ b/application/render/PageBuilder.php @@ -161,6 +161,7 @@ class PageBuilder $this->tpl->assign('formatter', $this->conf->get('formatter', 'default')); $this->tpl->assign('links_per_page', $this->session['LINKS_PER_PAGE'] ?? 20); + $this->tpl->assign('tags_separator', $this->conf->get('general.tags_separator', ' ')); // To be removed with a proper theme configuration. $this->tpl->assign('conf', $this->conf); diff --git a/assets/default/js/base.js b/assets/default/js/base.js index 66badfb2..e7bf4909 100644 --- a/assets/default/js/base.js +++ b/assets/default/js/base.js @@ -42,19 +42,20 @@ function refreshToken(basePath, callback) { xhr.send(); } -function createAwesompleteInstance(element, tags = []) { +function createAwesompleteInstance(element, separator, tags = []) { const awesome = new Awesomplete(Awesomplete.$(element)); - // Tags are separated by a space - awesome.filter = (text, input) => Awesomplete.FILTER_CONTAINS(text, input.match(/[^ ]*$/)[0]); + + // Tags are separated by separator + awesome.filter = (text, input) => Awesomplete.FILTER_CONTAINS(text, input.match(new RegExp(`[^${separator}]*$`))[0]); // Insert new selected tag in the input awesome.replace = (text) => { - const before = awesome.input.value.match(/^.+ \s*|/)[0]; - awesome.input.value = `${before}${text} `; + const before = awesome.input.value.match(new RegExp(`^.+${separator}+|`))[0]; + awesome.input.value = `${before}${text}${separator}`; }; // Highlight found items - awesome.item = (text, input) => Awesomplete.ITEM(text, input.match(/[^ ]*$/)[0]); + awesome.item = (text, input) => Awesomplete.ITEM(text, input.match(new RegExp(`[^${separator}]*$`))[0]); // Don't display already selected items - const reg = /(\w+) /g; + const reg = new RegExp(`/(\w+)${separator}/g`); let match; awesome.data = (item, input) => { while ((match = reg.exec(input))) { @@ -78,13 +79,14 @@ function createAwesompleteInstance(element, tags = []) { * @param selector CSS selector * @param tags Array of tags * @param instances List of existing awesomplete instances + * @param separator Tags separator character */ -function updateAwesompleteList(selector, tags, instances) { +function updateAwesompleteList(selector, tags, instances, separator) { if (instances.length === 0) { // First load: create Awesomplete instances const elements = document.querySelectorAll(selector); [...elements].forEach((element) => { - instances.push(createAwesompleteInstance(element, tags)); + instances.push(createAwesompleteInstance(element, separator, tags)); }); } else { // Update awesomplete tag list @@ -214,6 +216,8 @@ function init(description) { (() => { const basePath = document.querySelector('input[name="js_base_path"]').value; + const tagsSeparatorElement = document.querySelector('input[name="tags_separator"]'); + const tagsSeparator = tagsSeparatorElement ? tagsSeparatorElement.value || '\s' : '\s'; /** * Handle responsive menu. @@ -575,7 +579,7 @@ function init(description) { // Refresh awesomplete values existingTags = existingTags.map((tag) => (tag === fromtag ? totag : tag)); - awesomepletes = updateAwesompleteList('.rename-tag-input', existingTags, awesomepletes); + awesomepletes = updateAwesompleteList('.rename-tag-input', existingTags, awesomepletes, tagsSeparator); } }; xhr.send(`renametag=1&fromtag=${fromtagUrl}&totag=${encodeURIComponent(totag)}&token=${refreshedToken}`); @@ -615,14 +619,14 @@ function init(description) { refreshToken(basePath); existingTags = existingTags.filter((tagItem) => tagItem !== tag); - awesomepletes = updateAwesompleteList('.rename-tag-input', existingTags, awesomepletes); + awesomepletes = updateAwesompleteList('.rename-tag-input', existingTags, awesomepletes, tagsSeparator); } }); }); const autocompleteFields = document.querySelectorAll('input[data-multiple]'); [...autocompleteFields].forEach((autocompleteField) => { - awesomepletes.push(createAwesompleteInstance(autocompleteField)); + awesomepletes.push(createAwesompleteInstance(autocompleteField, tagsSeparator)); }); const exportForm = document.querySelector('#exportform'); diff --git a/assets/default/scss/shaarli.scss b/assets/default/scss/shaarli.scss index a7f091e9..ed774a9d 100644 --- a/assets/default/scss/shaarli.scss +++ b/assets/default/scss/shaarli.scss @@ -139,6 +139,16 @@ body, } } +.page-form, +.pure-alert { + code { + display: inline-block; + padding: 0 2px; + color: $dark-grey; + background-color: var(--background-color); + } +} + // Make pure-extras alert closable. .pure-alert-closable { .fa-times { diff --git a/doc/md/Shaarli-configuration.md b/doc/md/Shaarli-configuration.md index 99084728..b1326cce 100644 --- a/doc/md/Shaarli-configuration.md +++ b/doc/md/Shaarli-configuration.md @@ -74,6 +74,7 @@ Some settings can be configured directly from a web browser by accesing the `Too "timezone": "Europe\/Paris", "title": "My Shaarli", "header_link": "?" + "tags_separator": " " }, "dev": { "debug": false, @@ -153,6 +154,7 @@ _These settings should not be edited_ - **enable_async_metadata** (boolean): Retrieve external bookmark metadata asynchronously to prevent bookmark creation slowdown. - **retrieve_description** (boolean): If set to true, for every new Shaare Shaarli will try to retrieve the description and keywords from the HTML meta tags. - **root_url**: Overrides automatic discovery of Shaarli instance's URL (e.g.) `https://sub.domain.tld/shaarli-folder/`. +- **tags_separator**: Defines your tags separator (default: whitespace). ### Security diff --git a/index.php b/index.php index 4b5602ac..8fe86236 100644 --- a/index.php +++ b/index.php @@ -125,6 +125,7 @@ $app->group('/admin', function () { $this->post('/configure', '\Shaarli\Front\Controller\Admin\ConfigureController:save'); $this->get('/tags', '\Shaarli\Front\Controller\Admin\ManageTagController:index'); $this->post('/tags', '\Shaarli\Front\Controller\Admin\ManageTagController:save'); + $this->post('/tags/change-separator', '\Shaarli\Front\Controller\Admin\ManageTagController:changeSeparator'); $this->get('/add-shaare', '\Shaarli\Front\Controller\Admin\ShaareAddController:addShaare'); $this->get('/shaare', '\Shaarli\Front\Controller\Admin\ShaarePublishController:displayCreateForm'); $this->get('/shaare/{id:[0-9]+}', '\Shaarli\Front\Controller\Admin\ShaarePublishController:displayEditForm'); diff --git a/tests/bookmark/BookmarkFilterTest.php b/tests/bookmark/BookmarkFilterTest.php index 574d8e3f..835674f2 100644 --- a/tests/bookmark/BookmarkFilterTest.php +++ b/tests/bookmark/BookmarkFilterTest.php @@ -44,7 +44,7 @@ class BookmarkFilterTest extends TestCase self::$refDB->write(self::$testDatastore); $history = new History('sandbox/history.php'); self::$bookmarkService = new \FakeBookmarkService($conf, $history, $mutex, true); - self::$linkFilter = new BookmarkFilter(self::$bookmarkService->getBookmarks()); + self::$linkFilter = new BookmarkFilter(self::$bookmarkService->getBookmarks(), $conf); } /** diff --git a/tests/bookmark/BookmarkTest.php b/tests/bookmark/BookmarkTest.php index 4c1ae25d..cb91b26b 100644 --- a/tests/bookmark/BookmarkTest.php +++ b/tests/bookmark/BookmarkTest.php @@ -78,6 +78,23 @@ class BookmarkTest extends TestCase $this->assertTrue($bookmark->isNote()); } + /** + * Test fromArray() with a link with a custom tags separator + */ + public function testFromArrayCustomTagsSeparator() + { + $data = [ + 'id' => 1, + 'tags' => ['tag1', 'tag2', 'chair'], + ]; + + $bookmark = (new Bookmark())->fromArray($data, '@'); + $this->assertEquals($data['id'], $bookmark->getId()); + $this->assertEquals($data['tags'], $bookmark->getTags()); + $this->assertEquals('tag1@tag2@chair', $bookmark->getTagsString('@')); + } + + /** * Test validate() with a valid minimal bookmark */ @@ -252,7 +269,7 @@ class BookmarkTest extends TestCase { $bookmark = new Bookmark(); - $str = 'tag1 tag2 tag3.tag3-2, tag4 , -tag5 '; + $str = 'tag1 tag2 tag3.tag3-2 tag4 -tag5 '; $bookmark->setTagsString($str); $this->assertEquals( [ @@ -276,9 +293,9 @@ class BookmarkTest extends TestCase $array = [ 'tag1 ', ' tag2', - 'tag3.tag3-2,', - ', tag4', - ', ', + 'tag3.tag3-2', + ' tag4', + ' ', '-tag5 ', ]; $bookmark->setTags($array); diff --git a/tests/bookmark/LinkUtilsTest.php b/tests/bookmark/LinkUtilsTest.php index 3321242f..c2f9f930 100644 --- a/tests/bookmark/LinkUtilsTest.php +++ b/tests/bookmark/LinkUtilsTest.php @@ -247,7 +247,8 @@ class LinkUtilsTest extends TestCase $title, $desc, $keywords, - false + false, + ' ' ); $data = [ @@ -297,7 +298,8 @@ class LinkUtilsTest extends TestCase $title, $desc, $keywords, - false + false, + ' ' ); $data = [ @@ -330,7 +332,8 @@ class LinkUtilsTest extends TestCase $title, $desc, $keywords, - false + false, + ' ' ); $data = [ @@ -363,7 +366,8 @@ class LinkUtilsTest extends TestCase $title, $desc, $keywords, - false + false, + ' ' ); $data = [ @@ -428,7 +432,8 @@ class LinkUtilsTest extends TestCase $title, $desc, $keywords, - true + true, + ' ' ); $data = [ 'th=device-width">' @@ -574,6 +579,115 @@ class LinkUtilsTest extends TestCase $this->assertFalse(is_note('https://github.com/shaarli/Shaarli/?hi')); } + /** + * Test tags_str2array with whitespace separator. + */ + public function testTagsStr2ArrayWithSpaceSeparator(): void + { + $separator = ' '; + + static::assertSame(['tag1', 'tag2', 'tag3'], tags_str2array('tag1 tag2 tag3', $separator)); + static::assertSame(['tag1', 'tag2', 'tag3'], tags_str2array('tag1 tag2 tag3', $separator)); + static::assertSame(['tag1', 'tag2', 'tag3'], tags_str2array(' tag1 tag2 tag3 ', $separator)); + static::assertSame(['tag1@', 'tag2,', '.tag3'], tags_str2array(' tag1@ tag2, .tag3 ', $separator)); + static::assertSame([], tags_str2array('', $separator)); + static::assertSame([], tags_str2array(' ', $separator)); + static::assertSame([], tags_str2array(null, $separator)); + } + + /** + * Test tags_str2array with @ separator. + */ + public function testTagsStr2ArrayWithCharSeparator(): void + { + $separator = '@'; + + static::assertSame(['tag1', 'tag2', 'tag3'], tags_str2array('tag1@tag2@tag3', $separator)); + static::assertSame(['tag1', 'tag2', 'tag3'], tags_str2array('tag1@@@@tag2@@@@tag3', $separator)); + static::assertSame(['tag1', 'tag2', 'tag3'], tags_str2array('@@@tag1@@@tag2@@@@tag3@@', $separator)); + static::assertSame( + ['tag1#', 'tag2, and other', '.tag3'], + tags_str2array('@@@ tag1# @@@ tag2, and other @@@@.tag3@@', $separator) + ); + static::assertSame([], tags_str2array('', $separator)); + static::assertSame([], tags_str2array(' ', $separator)); + static::assertSame([], tags_str2array(null, $separator)); + } + + /** + * Test tags_array2str with ' ' separator. + */ + public function testTagsArray2StrWithSpaceSeparator(): void + { + $separator = ' '; + + static::assertSame('tag1 tag2 tag3', tags_array2str(['tag1', 'tag2', 'tag3'], $separator)); + static::assertSame('tag1, tag2@ tag3', tags_array2str(['tag1,', 'tag2@', 'tag3'], $separator)); + static::assertSame('tag1 tag2 tag3', tags_array2str([' tag1 ', 'tag2', 'tag3 '], $separator)); + static::assertSame('tag1 tag2 tag3', tags_array2str([' tag1 ', ' ', 'tag2', ' ', 'tag3 '], $separator)); + static::assertSame('tag1', tags_array2str([' tag1 '], $separator)); + static::assertSame('', tags_array2str([' '], $separator)); + static::assertSame('', tags_array2str([], $separator)); + static::assertSame('', tags_array2str(null, $separator)); + } + + /** + * Test tags_array2str with @ separator. + */ + public function testTagsArray2StrWithCharSeparator(): void + { + $separator = '@'; + + static::assertSame('tag1@tag2@tag3', tags_array2str(['tag1', 'tag2', 'tag3'], $separator)); + static::assertSame('tag1,@tag2@tag3', tags_array2str(['tag1,', 'tag2@', 'tag3'], $separator)); + static::assertSame( + 'tag1@tag2, and other@tag3', + tags_array2str(['@@@@ tag1@@@', ' @tag2, and other @', 'tag3@@@@'], $separator) + ); + static::assertSame('tag1@tag2@tag3', tags_array2str(['@@@tag1@@@', '@', 'tag2', '@@@', 'tag3@@@'], $separator)); + static::assertSame('tag1', tags_array2str(['@@@@tag1@@@@'], $separator)); + static::assertSame('', tags_array2str(['@@@'], $separator)); + static::assertSame('', tags_array2str([], $separator)); + static::assertSame('', tags_array2str(null, $separator)); + } + + /** + * Test tags_array2str with @ separator. + */ + public function testTagsFilterWithSpaceSeparator(): void + { + $separator = ' '; + + static::assertSame(['tag1', 'tag2', 'tag3'], tags_filter(['tag1', 'tag2', 'tag3'], $separator)); + static::assertSame(['tag1,', 'tag2@', 'tag3'], tags_filter(['tag1,', 'tag2@', 'tag3'], $separator)); + static::assertSame(['tag1', 'tag2', 'tag3'], tags_filter([' tag1 ', 'tag2', 'tag3 '], $separator)); + static::assertSame(['tag1', 'tag2', 'tag3'], tags_filter([' tag1 ', ' ', 'tag2', ' ', 'tag3 '], $separator)); + static::assertSame(['tag1'], tags_filter([' tag1 '], $separator)); + static::assertSame([], tags_filter([' '], $separator)); + static::assertSame([], tags_filter([], $separator)); + static::assertSame([], tags_filter(null, $separator)); + } + + /** + * Test tags_array2str with @ separator. + */ + public function testTagsArrayFilterWithSpaceSeparator(): void + { + $separator = '@'; + + static::assertSame(['tag1', 'tag2', 'tag3'], tags_filter(['tag1', 'tag2', 'tag3'], $separator)); + static::assertSame(['tag1,', 'tag2#', 'tag3'], tags_filter(['tag1,', 'tag2#', 'tag3'], $separator)); + static::assertSame( + ['tag1', 'tag2, and other', 'tag3'], + tags_filter(['@@@@ tag1@@@', ' @tag2, and other @', 'tag3@@@@'], $separator) + ); + static::assertSame(['tag1', 'tag2', 'tag3'], tags_filter(['@@@tag1@@@', '@', 'tag2', '@@@', 'tag3@@@'], $separator)); + static::assertSame(['tag1'], tags_filter(['@@@@tag1@@@@'], $separator)); + static::assertSame([], tags_filter(['@@@'], $separator)); + static::assertSame([], tags_filter([], $separator)); + static::assertSame([], tags_filter(null, $separator)); + } + /** * Util function to build an hashtag link. * diff --git a/tests/front/controller/admin/ManageTagControllerTest.php b/tests/front/controller/admin/ManageTagControllerTest.php index 8a0ff7a9..af6f273f 100644 --- a/tests/front/controller/admin/ManageTagControllerTest.php +++ b/tests/front/controller/admin/ManageTagControllerTest.php @@ -6,6 +6,7 @@ namespace Shaarli\Front\Controller\Admin; use Shaarli\Bookmark\Bookmark; use Shaarli\Bookmark\BookmarkFilter; +use Shaarli\Config\ConfigManager; use Shaarli\Front\Exception\WrongTokenException; use Shaarli\Security\SessionManager; use Shaarli\TestCase; @@ -44,9 +45,32 @@ class ManageTagControllerTest extends TestCase static::assertSame('changetag', (string) $result->getBody()); static::assertSame('fromtag', $assignedVariables['fromtag']); + static::assertSame('@', $assignedVariables['tags_separator']); static::assertSame('Manage tags - Shaarli', $assignedVariables['pagetitle']); } + /** + * Test displaying manage tag page + */ + public function testIndexWhitespaceSeparator(): void + { + $assignedVariables = []; + $this->assignTemplateVars($assignedVariables); + + $this->container->conf = $this->createMock(ConfigManager::class); + $this->container->conf->method('get')->willReturnCallback(function (string $key) { + return $key === 'general.tags_separator' ? ' ' : $key; + }); + + $request = $this->createMock(Request::class); + $response = new Response(); + + $this->controller->index($request, $response); + + static::assertSame(' ', $assignedVariables['tags_separator']); + static::assertSame('whitespace', $assignedVariables['tags_separator_desc']); + } + /** * Test posting a tag update - rename tag - valid info provided. */ @@ -269,4 +293,116 @@ class ManageTagControllerTest extends TestCase static::assertArrayNotHasKey(SessionManager::KEY_SUCCESS_MESSAGES, $session); static::assertSame(['Invalid tags provided.'], $session[SessionManager::KEY_WARNING_MESSAGES]); } + + /** + * Test changeSeparator to '#': redirection + success message. + */ + public function testChangeSeparatorValid(): void + { + $toSeparator = '#'; + + $session = []; + $this->assignSessionVars($session); + + $request = $this->createMock(Request::class); + $request + ->expects(static::atLeastOnce()) + ->method('getParam') + ->willReturnCallback(function (string $key) use ($toSeparator): ?string { + return $key === 'separator' ? $toSeparator : $key; + }) + ; + $response = new Response(); + + $this->container->conf + ->expects(static::once()) + ->method('set') + ->with('general.tags_separator', $toSeparator, true, true) + ; + + $result = $this->controller->changeSeparator($request, $response); + + static::assertSame(302, $result->getStatusCode()); + static::assertSame(['/subfolder/admin/tags'], $result->getHeader('location')); + + static::assertArrayNotHasKey(SessionManager::KEY_ERROR_MESSAGES, $session); + static::assertArrayNotHasKey(SessionManager::KEY_WARNING_MESSAGES, $session); + static::assertArrayHasKey(SessionManager::KEY_SUCCESS_MESSAGES, $session); + static::assertSame( + ['Your tags separator setting has been updated!'], + $session[SessionManager::KEY_SUCCESS_MESSAGES] + ); + } + + /** + * Test changeSeparator to '#@' (too long): redirection + error message. + */ + public function testChangeSeparatorInvalidTooLong(): void + { + $toSeparator = '#@'; + + $session = []; + $this->assignSessionVars($session); + + $request = $this->createMock(Request::class); + $request + ->expects(static::atLeastOnce()) + ->method('getParam') + ->willReturnCallback(function (string $key) use ($toSeparator): ?string { + return $key === 'separator' ? $toSeparator : $key; + }) + ; + $response = new Response(); + + $this->container->conf->expects(static::never())->method('set'); + + $result = $this->controller->changeSeparator($request, $response); + + static::assertSame(302, $result->getStatusCode()); + static::assertSame(['/subfolder/admin/tags'], $result->getHeader('location')); + + static::assertArrayNotHasKey(SessionManager::KEY_SUCCESS_MESSAGES, $session); + static::assertArrayNotHasKey(SessionManager::KEY_WARNING_MESSAGES, $session); + static::assertArrayHasKey(SessionManager::KEY_ERROR_MESSAGES, $session); + static::assertSame( + ['Tags separator must be a single character.'], + $session[SessionManager::KEY_ERROR_MESSAGES] + ); + } + + /** + * Test changeSeparator to '#@' (too long): redirection + error message. + */ + public function testChangeSeparatorInvalidReservedCharacter(): void + { + $toSeparator = '*'; + + $session = []; + $this->assignSessionVars($session); + + $request = $this->createMock(Request::class); + $request + ->expects(static::atLeastOnce()) + ->method('getParam') + ->willReturnCallback(function (string $key) use ($toSeparator): ?string { + return $key === 'separator' ? $toSeparator : $key; + }) + ; + $response = new Response(); + + $this->container->conf->expects(static::never())->method('set'); + + $result = $this->controller->changeSeparator($request, $response); + + static::assertSame(302, $result->getStatusCode()); + static::assertSame(['/subfolder/admin/tags'], $result->getHeader('location')); + + static::assertArrayNotHasKey(SessionManager::KEY_SUCCESS_MESSAGES, $session); + static::assertArrayNotHasKey(SessionManager::KEY_WARNING_MESSAGES, $session); + static::assertArrayHasKey(SessionManager::KEY_ERROR_MESSAGES, $session); + static::assertStringStartsWith( + 'These characters are reserved and can\'t be used as tags separator', + $session[SessionManager::KEY_ERROR_MESSAGES][0] + ); + } } diff --git a/tests/front/controller/admin/ShaarePublishControllerTest/DisplayCreateFormTest.php b/tests/front/controller/admin/ShaarePublishControllerTest/DisplayCreateFormTest.php index f20b1def..964773da 100644 --- a/tests/front/controller/admin/ShaarePublishControllerTest/DisplayCreateFormTest.php +++ b/tests/front/controller/admin/ShaarePublishControllerTest/DisplayCreateFormTest.php @@ -101,7 +101,7 @@ class DisplayCreateFormTest extends TestCase static::assertSame($expectedUrl, $assignedVariables['link']['url']); static::assertSame($remoteTitle, $assignedVariables['link']['title']); static::assertSame($remoteDesc, $assignedVariables['link']['description']); - static::assertSame($remoteTags, $assignedVariables['link']['tags']); + static::assertSame($remoteTags . ' ', $assignedVariables['link']['tags']); static::assertFalse($assignedVariables['link']['private']); static::assertTrue($assignedVariables['link_is_new']); @@ -192,7 +192,7 @@ class DisplayCreateFormTest extends TestCase 'post' => 'http://url.tld/other?part=3&utm_ad=pay#hash', 'title' => 'Provided Title', 'description' => 'Provided description.', - 'tags' => 'abc def', + 'tags' => 'abc@def', 'private' => '1', 'source' => 'apps', ]; @@ -216,7 +216,7 @@ class DisplayCreateFormTest extends TestCase static::assertSame($expectedUrl, $assignedVariables['link']['url']); static::assertSame($parameters['title'], $assignedVariables['link']['title']); static::assertSame($parameters['description'], $assignedVariables['link']['description']); - static::assertSame($parameters['tags'], $assignedVariables['link']['tags']); + static::assertSame($parameters['tags'] . '@', $assignedVariables['link']['tags']); static::assertTrue($assignedVariables['link']['private']); static::assertTrue($assignedVariables['link_is_new']); static::assertSame($parameters['source'], $assignedVariables['source']); @@ -360,7 +360,7 @@ class DisplayCreateFormTest extends TestCase static::assertSame($expectedUrl, $assignedVariables['link']['url']); static::assertSame($title, $assignedVariables['link']['title']); static::assertSame($description, $assignedVariables['link']['description']); - static::assertSame(implode(' ', $tags), $assignedVariables['link']['tags']); + static::assertSame(implode('@', $tags) . '@', $assignedVariables['link']['tags']); static::assertTrue($assignedVariables['link']['private']); static::assertSame($createdAt, $assignedVariables['link']['created']); } diff --git a/tests/front/controller/admin/ShaarePublishControllerTest/DisplayEditFormTest.php b/tests/front/controller/admin/ShaarePublishControllerTest/DisplayEditFormTest.php index da393e49..738cea12 100644 --- a/tests/front/controller/admin/ShaarePublishControllerTest/DisplayEditFormTest.php +++ b/tests/front/controller/admin/ShaarePublishControllerTest/DisplayEditFormTest.php @@ -74,7 +74,7 @@ class DisplayEditFormTest extends TestCase static::assertSame($url, $assignedVariables['link']['url']); static::assertSame($title, $assignedVariables['link']['title']); static::assertSame($description, $assignedVariables['link']['description']); - static::assertSame(implode(' ', $tags), $assignedVariables['link']['tags']); + static::assertSame(implode('@', $tags) . '@', $assignedVariables['link']['tags']); static::assertTrue($assignedVariables['link']['private']); static::assertSame($createdAt, $assignedVariables['link']['created']); } diff --git a/tests/front/controller/visitor/BookmarkListControllerTest.php b/tests/front/controller/visitor/BookmarkListControllerTest.php index 5cbc8c73..dec938f2 100644 --- a/tests/front/controller/visitor/BookmarkListControllerTest.php +++ b/tests/front/controller/visitor/BookmarkListControllerTest.php @@ -173,7 +173,7 @@ class BookmarkListControllerTest extends TestCase $request = $this->createMock(Request::class); $request->method('getParam')->willReturnCallback(function (string $key) { if ('searchtags' === $key) { - return 'abc def'; + return 'abc@def'; } if ('searchterm' === $key) { return 'ghi jkl'; @@ -204,7 +204,7 @@ class BookmarkListControllerTest extends TestCase ->expects(static::once()) ->method('search') ->with( - ['searchtags' => 'abc def', 'searchterm' => 'ghi jkl'], + ['searchtags' => 'abc@def', 'searchterm' => 'ghi jkl'], 'private', false, true @@ -222,7 +222,7 @@ class BookmarkListControllerTest extends TestCase static::assertSame('linklist', (string) $result->getBody()); static::assertSame('Search: ghi jkl [abc] [def] - Shaarli', $assignedVariables['pagetitle']); - static::assertSame('?page=2&searchterm=ghi+jkl&searchtags=abc+def', $assignedVariables['previous_page_url']); + static::assertSame('?page=2&searchterm=ghi+jkl&searchtags=abc%40def', $assignedVariables['previous_page_url']); } /** diff --git a/tests/front/controller/visitor/FrontControllerMockHelper.php b/tests/front/controller/visitor/FrontControllerMockHelper.php index fc0bb7d1..02229f68 100644 --- a/tests/front/controller/visitor/FrontControllerMockHelper.php +++ b/tests/front/controller/visitor/FrontControllerMockHelper.php @@ -41,6 +41,10 @@ trait FrontControllerMockHelper // Config $this->container->conf = $this->createMock(ConfigManager::class); $this->container->conf->method('get')->willReturnCallback(function (string $parameter, $default) { + if ($parameter === 'general.tags_separator') { + return '@'; + } + return $default === null ? $parameter : $default; }); diff --git a/tests/front/controller/visitor/TagCloudControllerTest.php b/tests/front/controller/visitor/TagCloudControllerTest.php index 9305612e..4915573d 100644 --- a/tests/front/controller/visitor/TagCloudControllerTest.php +++ b/tests/front/controller/visitor/TagCloudControllerTest.php @@ -100,7 +100,7 @@ class TagCloudControllerTest extends TestCase ->with() ->willReturnCallback(function (string $key): ?string { if ('searchtags' === $key) { - return 'ghi def'; + return 'ghi@def'; } return null; @@ -131,7 +131,7 @@ class TagCloudControllerTest extends TestCase ->withConsecutive(['render_tagcloud']) ->willReturnCallback(function (string $hook, array $data, array $param): array { if ('render_tagcloud' === $hook) { - static::assertSame('ghi def', $data['search_tags']); + static::assertSame('ghi@def@', $data['search_tags']); static::assertCount(1, $data['tags']); static::assertArrayHasKey('loggedin', $param); @@ -147,7 +147,7 @@ class TagCloudControllerTest extends TestCase static::assertSame('tag.cloud', (string) $result->getBody()); static::assertSame('ghi def - Tag cloud - Shaarli', $assignedVariables['pagetitle']); - static::assertSame('ghi def', $assignedVariables['search_tags']); + static::assertSame('ghi@def@', $assignedVariables['search_tags']); static::assertCount(1, $assignedVariables['tags']); static::assertArrayHasKey('abc', $assignedVariables['tags']); @@ -277,7 +277,7 @@ class TagCloudControllerTest extends TestCase ->with() ->willReturnCallback(function (string $key): ?string { if ('searchtags' === $key) { - return 'ghi def'; + return 'ghi@def'; } elseif ('sort' === $key) { return 'alpha'; } @@ -310,7 +310,7 @@ class TagCloudControllerTest extends TestCase ->withConsecutive(['render_taglist']) ->willReturnCallback(function (string $hook, array $data, array $param): array { if ('render_taglist' === $hook) { - static::assertSame('ghi def', $data['search_tags']); + static::assertSame('ghi@def@', $data['search_tags']); static::assertCount(1, $data['tags']); static::assertArrayHasKey('loggedin', $param); @@ -326,7 +326,7 @@ class TagCloudControllerTest extends TestCase static::assertSame('tag.list', (string) $result->getBody()); static::assertSame('ghi def - Tag list - Shaarli', $assignedVariables['pagetitle']); - static::assertSame('ghi def', $assignedVariables['search_tags']); + static::assertSame('ghi@def@', $assignedVariables['search_tags']); static::assertCount(1, $assignedVariables['tags']); static::assertSame(3, $assignedVariables['tags']['abc']); } diff --git a/tests/front/controller/visitor/TagControllerTest.php b/tests/front/controller/visitor/TagControllerTest.php index 750ea02d..5a556c6d 100644 --- a/tests/front/controller/visitor/TagControllerTest.php +++ b/tests/front/controller/visitor/TagControllerTest.php @@ -50,7 +50,7 @@ class TagControllerTest extends TestCase static::assertInstanceOf(Response::class, $result); static::assertSame(302, $result->getStatusCode()); - static::assertSame(['/controller/?searchtags=def+abc'], $result->getHeader('location')); + static::assertSame(['/controller/?searchtags=def%40abc'], $result->getHeader('location')); } public function testAddTagWithoutRefererAndExistingSearch(): void @@ -80,7 +80,7 @@ class TagControllerTest extends TestCase static::assertInstanceOf(Response::class, $result); static::assertSame(302, $result->getStatusCode()); - static::assertSame(['/controller/?searchtags=def+abc'], $result->getHeader('location')); + static::assertSame(['/controller/?searchtags=def%40abc'], $result->getHeader('location')); } public function testAddTagResetPagination(): void @@ -96,7 +96,7 @@ class TagControllerTest extends TestCase static::assertInstanceOf(Response::class, $result); static::assertSame(302, $result->getStatusCode()); - static::assertSame(['/controller/?searchtags=def+abc'], $result->getHeader('location')); + static::assertSame(['/controller/?searchtags=def%40abc'], $result->getHeader('location')); } public function testAddTagWithRefererAndEmptySearch(): void diff --git a/tests/netscape/BookmarkImportTest.php b/tests/netscape/BookmarkImportTest.php index c526d5c8..6856ebca 100644 --- a/tests/netscape/BookmarkImportTest.php +++ b/tests/netscape/BookmarkImportTest.php @@ -531,7 +531,7 @@ class BookmarkImportTest extends TestCase { $post = array( 'privacy' => 'public', - 'default_tags' => 'tag1,tag2 tag3' + 'default_tags' => 'tag1 tag2 tag3' ); $files = file2array('netscape_basic.htm'); $this->assertStringMatchesFormat( @@ -552,7 +552,7 @@ class BookmarkImportTest extends TestCase { $post = array( 'privacy' => 'public', - 'default_tags' => 'tag1&,tag2 "tag3"' + 'default_tags' => 'tag1& tag2 "tag3"' ); $files = file2array('netscape_basic.htm'); $this->assertStringMatchesFormat( @@ -572,6 +572,43 @@ class BookmarkImportTest extends TestCase ); } + /** + * Add user-specified tags to all imported bookmarks + */ + public function testSetDefaultTagsWithCustomSeparator() + { + $separator = '@'; + $this->conf->set('general.tags_separator', $separator); + $post = [ + 'privacy' => 'public', + 'default_tags' => 'tag1@tag2@tag3@multiple words tag' + ]; + $files = file2array('netscape_basic.htm'); + $this->assertStringMatchesFormat( + 'File netscape_basic.htm (482 bytes) was successfully processed in %d seconds:' + .' 2 bookmarks imported, 0 bookmarks overwritten, 0 bookmarks skipped.', + $this->netscapeBookmarkUtils->import($post, $files) + ); + $this->assertEquals(2, $this->bookmarkService->count()); + $this->assertEquals(0, $this->bookmarkService->count(BookmarkFilter::$PRIVATE)); + $this->assertEquals( + 'tag1@tag2@tag3@multiple words tag@private@secret', + $this->bookmarkService->get(0)->getTagsString($separator) + ); + $this->assertEquals( + ['tag1', 'tag2', 'tag3', 'multiple words tag', 'private', 'secret'], + $this->bookmarkService->get(0)->getTags() + ); + $this->assertEquals( + 'tag1@tag2@tag3@multiple words tag@public@hello@world', + $this->bookmarkService->get(1)->getTagsString($separator) + ); + $this->assertEquals( + ['tag1', 'tag2', 'tag3', 'multiple words tag', 'public', 'hello', 'world'], + $this->bookmarkService->get(1)->getTags() + ); + } + /** * Ensure each imported bookmark has a unique id * diff --git a/tpl/default/changetag.html b/tpl/default/changetag.html index a5fbd31e..13b7f24a 100644 --- a/tpl/default/changetag.html +++ b/tpl/default/changetag.html @@ -36,6 +36,29 @@

{'You can also edit tags in the'|t} {'tag list'|t}.

+ +
+
+
+

{"Change tags separator"|t}

+
+

+ {'Your current tag separator is'|t} {$tags_separator}{if="!empty($tags_separator_desc)"} ({$tags_separator_desc}){/if}. +

+
+ +
+ +
+ +
+

+ {'Note that hashtags won\'t fully work with a non-whitespace separator.'|t} +

+
+
+
{include="page.footer"} diff --git a/tpl/default/linklist.html b/tpl/default/linklist.html index e1115d49..7208a3b6 100644 --- a/tpl/default/linklist.html +++ b/tpl/default/linklist.html @@ -90,7 +90,7 @@ {'for'|t} {$search_term} {/if} {if="!empty($search_tags)"} - {$exploded_tags=explode(' ', $search_tags)} + {$exploded_tags=tags_str2array($search_tags, $tags_separator)} {'tagged'|t} {loop="$exploded_tags"} diff --git a/tpl/default/page.footer.html b/tpl/default/page.footer.html index 964ffff1..58ca18c5 100644 --- a/tpl/default/page.footer.html +++ b/tpl/default/page.footer.html @@ -18,8 +18,6 @@
- - {loop="$plugins_footer.endofpage"} {$value} {/loop} @@ -41,4 +39,7 @@ + + + diff --git a/tpl/default/tag.cloud.html b/tpl/default/tag.cloud.html index c067e1d4..01b50b02 100644 --- a/tpl/default/tag.cloud.html +++ b/tpl/default/tag.cloud.html @@ -48,7 +48,7 @@
{loop="tags"} - {$key}{$key}{$value.count} {loop="$value.tag_plugin"} {$value}