There are quite some places where you have a code like this in views:

class Foo ... {
  function render(...) {

These ones should all get the public identifier.

Noticed as part of #2030293: View preview is broken in UI if a pager has to be displayed

#3 public-2036087-3.patch40.36 KBYesCT
PASSED: [[SimpleTest]]: [MySQL] 56,923 pass(es). View
Members fund testing for the Drupal project. Drupal Association Learn more


dawehner’s picture

Issue tags: +Novice


YesCT’s picture

Assigned: Unassigned » YesCT

note, there is also: > core/lib/Drupal/Component/Diff/DiffEngine.php:1244: function render() {
separate issue?

YesCT’s picture

Assigned: YesCT » Unassigned
Status: Active » Needs review
40.36 KB
PASSED: [[SimpleTest]]: [MySQL] 56,923 pass(es). View

done by

ag " function render\(" core | grep -v public | grep view | cut -f1 -d":" > ~/filelist.txt
$ cat ~/ 
for x in `cat ~/filelist.txt `
sed "s/ function render(/ public function render(/" $x > temp
cat temp > $x

there were 64 files that were effected.

git diff | grep diff | wc -l
dawehner’s picture

Status: Needs review » Reviewed & tested by the community

Awesome work!

alexpott’s picture

Committed 31efff2 and pushed to 8.x. Thanks!

mondrake’s picture

Status: Reviewed & tested by the community » Fixed

So this is fixed

benjifisher’s picture

Do we care that AreaPluginBase and PagerPluginBase have inconsistent declarations? The former has

  public abstract function render($empty = FALSE);

and the latter has

  public function render($input) { }

BTW, what is the "ag" utility mentioned in #3, some "grep -r" variant?

dawehner’s picture

Regarding ag: Have a look at

Yes it is okay to have different arguments on the render method, as they do something different.

benjifisher’s picture

Thanks for the pointer to ag.

I was wondering more about declaring just one as abstract. Both classes are declared as abstract, so why does one method get declared as abstract and not the other? I do not know PHP well enough to know whether this is a significant difference, or entirely equivalent. If the latter, then consistency would be clearer.

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

Anonymous’s picture

Issue summary: View changes

added the parent issue