Part of #1929270: [meta] Drupal-agnostic components should not be calling Drupal functions

CommentFileSizeAuthor
#8 drupal-2043757-8.patch5.57 KBalan d.
#5 drupal-2043757-5.patch4.26 KBdawehner
#5 interdiff.txt2.46 KBdawehner
#1 drupal-2043757-1.patch3.07 KBdawehner

Comments

dawehner’s picture

dawehner
Primary language German
commented
StatusFileSize
new drupal-2043757-1.patch3.07 KB

There we go.

But it seems impossible to convert the drupal_render call on the longrun, at least it won't be a component.

ParisLiakos’s picture

ParisLiakos commented
Status: Active » Needs review
catch’s picture

catch
he/him
Primary language English
commented
Component: database system » base system

Should we move this to core here or a new patch for that?

dawehner’s picture

dawehner
Primary language German
commented

Should we move this to core here or a new patch for that?

What we could do is to let everything beside the render method resists on components and make this function abstract, so other people can just use it
in there own render system. Drupal than just uses the component and implements the rendering in the core namespace.

dawehner’s picture

dawehner
Primary language German
commented
StatusFileSize
new interdiff.txt2.46 KB
new drupal-2043757-5.patch4.26 KB

OOH I just realized that this DiffEngine file is full classes but also that DrupalDiffInline don't seem to be actually used.

alan d.’s picture

alan d. commented

Cut n paste verbatim from the Diff 7.x module to Drupal 8.x :)

function diff_get_inline($a, $b) {
  $diff = new DrupalDiffInline($a, $b);
  return $diff->render();
}
catch’s picture

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

Looks good to me.

alan d.’s picture

alan d. commented
Status: Reviewed & tested by the community » Needs review
StatusFileSize
new drupal-2043757-8.patch5.57 KB

What is the point of making DrupalDiffInline abstract when it is not used in core? Wouldn't it be better to remove it completely? I'd assume that the Diff module will be the only module that uses this functionality.

The attached patch does this ;)

catch’s picture

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

That works too. Also opened #2113573: Convert USE_ASSERTS to a class constant.

catch’s picture

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

Committed/pushed to 8.x, thanks!

Status: Fixed » Closed (fixed)

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