Merge two regex in one to define version number

Created on 4 October 2016, about 8 years ago
Updated 3 March 2023, over 1 year ago

Problem/Motivation

Two regex could be merged in one in locale_translation_build_projects().

      if (preg_match("/^(\d+\.x-\d+\.).*$/", $data['info']['version'], $matches)) {
        // Example matches: 8.x-1.x-dev, 8.x-1.0-alpha1+5-dev => 8.x-1.x
        $data['info']['version'] = $matches[1] . 'x';
      }
      elseif (preg_match("/^(\d+\.\d+\.).*$/", $data['info']['version'], $matches)) {
        // Example match: 8.0.0-dev => 8.0.x (Drupal core)
        $data['info']['version'] = $matches[1] . 'x';
      }

Minor but remove 3 lines of code.

Proposed resolution

Merge regex in one.

      if (preg_match("/^(\d+\.(x-)?\d+\.).*$/", $data['info']['version'], $matches)) {
        // Example matches:
        // * 8.x-1.x-dev, 8.x-1.0-alpha1+5-dev => 8.x-1.x
        // * 8.0.0-dev => 8.0.x (Drupal core)
        $data['info']['version'] = $matches[1] . 'x';
      }
๐Ÿ“Œ Task
Status

Needs work

Version

10.1 โœจ

Component
Localeย  โ†’

Last updated 6 days ago

Created by

๐Ÿ‡ซ๐Ÿ‡ทFrance goz

Live updates comments and jobs are added and updated live.
  • Quick fix

    Very small and simple change. Preferred over Quickfix.

Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia nikhil_110

    Attached patch against Drupal 10.1.x

  • First commit to issue fork.
  • Assigned to urvashi_vora
  • Status changed to Needs review over 1 year ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia urvashi_vora Madhya Pradesh, India

    Setting the Issue status to "Needs Review" since @Nikhil_110 provided a patch in #25.

    I will review this patch.

    Thanks

  • Issue was unassigned.
  • Status changed to RTBC over 1 year ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia urvashi_vora Madhya Pradesh, India

    Hello,

    I reviewed #25, and the patch applied cleanly for me.

    urvasi@urvasi-Inspiron-15-3552:/var/www/html/contribution/drupal-2810917$ git apply -v 2810917-25.patch
    Checking patch core/modules/locale/locale.compare.inc...
    Applied patch core/modules/locale/locale.compare.inc cleanly.
    

    I can confirm that after applying the patch

    if (preg_match("/^(\d+\.x-\d+\.).*$/", $data['info']['version'], $matches)) {
      // Example matches: 8.x-1.x-dev, 8.x-1.0-alpha1+5-dev => 8.x-1.x
      $data['info']['version'] = $matches[1] . 'x';
    }
    elseif (preg_match("/^(\d+\.\d+\.).*$/", $data['info']['version'], $matches)) {
      // Example match: 8.0.0-dev => 8.0.x (Drupal core)
    

    is replaced by

    if (preg_match("/^(\d+\.(x-)?\d+\.).*$/", $data['info']['version'], $matches)) {
      // Example matches:
      // * 8.x-1.x-dev, 8.x-1.0-alpha1+5-dev => 8.x-1.x
      // * 8.0.0-dev => 8.0.x (Drupal core)
      $data['info']['version'] = $matches[1] . 'x';
    }
    

    Steps performed while reviewing:-
    1. Taken clone of issue branch
    2. Reviewed locale.compare.inc file as per #25.
    3. From line number 59 to 66 is reduced as per issue summary.

    Test Result:- Pass.
    Moving it to RTBC, because the patch provided in #25 looks good to me.

    Please change the status if you find this inappropriate.

    Thank You!

  • Status changed to Needs work over 1 year ago
  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom longwave UK

    Again, the question from #6 still needs answering: is this regex covered by any existing tests?

Production build 0.71.5 2024