Files: 
CommentFileSizeAuthor
#8 drupal-2043757-8.patch5.57 KBAlan D.
PASSED: [[SimpleTest]]: [MySQL] 59,179 pass(es).
[ View ]
#5 drupal-2043757-5.patch4.26 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 58,977 pass(es).
[ View ]
#5 interdiff.txt2.46 KBdawehner
#1 drupal-2043757-1.patch3.07 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 59,240 pass(es).
[ View ]

Comments

dawehner’s picture

StatusFileSize
new3.07 KB
PASSED: [[SimpleTest]]: [MySQL] 59,240 pass(es).
[ View ]

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

Status:Active» Needs review
catch’s picture

Component:database system» base system

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

dawehner’s picture

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

StatusFileSize
new2.46 KB
new4.26 KB
PASSED: [[SimpleTest]]: [MySQL] 58,977 pass(es).
[ View ]

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

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

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

Status:Needs review» Reviewed & tested by the community

Looks good to me.

Alan D.’s picture

Status:Reviewed & tested by the community» Needs review
StatusFileSize
new5.57 KB
PASSED: [[SimpleTest]]: [MySQL] 59,179 pass(es).
[ View ]

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

Status:Needs review» Reviewed & tested by the community
catch’s picture

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.