Contrib.social

Merge two regex in one to define version number

Open on Drupal.org →
Created on 4 October 2016, about 8 years ago
Updated 3 March 2023, almost 2 years 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 10 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.

  • Comment almost 2 years ago →
    🇮🇳India nikhil_110

    Attached patch against Drupal 10.1.x

  • First commit to issue fork.
  • Assigned to urvashi_vora
  • Status changed to Needs review almost 2 years ago
  • Comment almost 2 years 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 almost 2 years ago
  • Comment almost 2 years 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 almost 2 years ago
  • Comment almost 2 years ago →
    🇬🇧United Kingdom longwave UK

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

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024