Problem/Motivation

The diff engine allows you to configure how much context you want to display using the diff_context_lines_leading and diff_context_lines_trailing variables, but the variable system will not be available in Drupal 8.

Proposed resolution

Convert the following lines to use CMI:
core/lib/Drupal/Component/Diff/DiffEngine.php
1099 $this->leading_context_lines = variable_get('diff_context_lines_leading', 2);
1100 $this->trailing_context_lines = variable_get('diff_context_lines_trailing', 2);

There is no UI to set these variables.

Remaining tasks

User interface changes

API changes

This is a child of #1775842: [meta] Convert all variables to state and/or config systems

CommentFileSizeAuthor
#6 2102499-6.patch894 bytesswentel
#3 2102499-3.patch734 bytesswentel
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented

If this is going to stay a component, it should use settings rather than state.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Title: Convert DiffEngine to CMI » Convert DiffEngine to settings
Priority: Normal » Critical

Also related to #1929270: [meta] Drupal-agnostic components should not be calling Drupal functions.

swentel’s picture

swentel CreditAttribution: swentel commented
Status: Active » Needs review
FileSize
2102499-3.patch734 bytes
ianthomas_uk’s picture

ianthomas_uk
he/him
Primary language English
Location Brighton, UK
CreditAttribution: ianthomas_uk commented
Status: Needs review » Needs work

That's still calling a Drupal function from Drupal-agnostic code (see #2)

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented

settings() is just wrapper for Settings::getSingleton() (which is in Component\Utility), we can use that.

swentel’s picture

swentel CreditAttribution: swentel commented
Status: Needs work » Needs review
FileSize
2102499-6.patch894 bytes

Ok, this then.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented
Status: Needs review » Reviewed & tested by the community

Ok, let's do this. I don't want to look at this file anymore ;)

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.x, thanks!

swentel’s picture

swentel CreditAttribution: swentel commented
Status: Fixed » Reviewed & tested by the community

Looks like you forgot to push.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

Automatically closed -- issue fixed for 2 weeks with no activity.