Separate form and comments snippets

[jenstornell]

For me the form and the comments are two different things. Maybe separate them? To make it backward compatible, do it like this:

comments.php

snippet('commentlist');
snippet('form');

Then for people who still uses comments.php it will still work. It would now be possible to only override for the form or only the commentlist.

[florianpircher]

This seams like a good idea. Keeping comments.php, which then includes comments-form.php and comments-list.php. Currently there is only the grouped file because I wanted a one-liner installation of the plugin into any site.

I still have to think about some complications, such as a the shared $comments and $status variables, which would have to be defined in one and only one of the two files. Maybe I’ll add a check in both files (comments-list.php and comments-form.php) which checks whether the variables are already defined.

[jenstornell]

I think a check in both files would be fine.

Here is an alternative solution:

https://github.com/jenstornell/kirby-secrets/wiki/global-data

Put the object in kirby()->set('option', 'comments', $comments); by the plugin just before the snippet loads.

Get it with $comments = kirby()->get('option', 'comments'); in the snippet.

Maybe it's overkill, but the snippets would be a row shorter.

I think both ways are fine!

[florianpircher]

I try to keep the snippets simple and easy to understand, therefore including code from a repo called kirby-secrets is unadvisable 😄. Thank you for your approval of the variable-check strategy, I am going to implement this soon.

EDIT 2017-03-23: https://github.com/Addpixel/KirbyComments/issues/17#issuecomment-288883352

[jenstornell]

https://github.com/jenstornell/kirby-secrets is my second most liked repository, so it's not that bad. Look at it and you might learn something on the way. =)

Another approach would be like this:

$comments = new comments();
snippet('comments-list', $comments);
snippet('comments-form', $comments);

I just throw some alternatives at you but I still think your alternative is fine.

[florianpircher]

While this approach is much more elegant than the double-check, it does not handle the $status variable for error reporting and it is more verbose.

The main drawback however, I think, is that it is not consistent.

Snippets Parameter

Use Case: comments snippet

templates/page.php

// no parameter requirement because that would be a breaking change
snippet('comments');

Use Case: comments-list and comments-form snippet

templates/page.php

$comments = new Comments();
$comments_status = $comments->process();
// ...
snippet('comments-list', array('comments' => $comments, 'comments_status' => $comments_status));
// ...
snippet('comments-form', array('comments' => $comments, 'comments_status' => $comments_status));

Double Check

Use Case: comments snippet

templates/page.php

snippet('comments');

Use Case: comments-list and comments-form snippet

templates/page.php

snippet('comments-list');
// ...
snippet('comments-form');

---

Letting the computer do all the thinking results in a shorter, simpler and more consistent syntax. Come to think of it: I like computers doing all the thinking.

Still, thank you for your efforts in exploring more elegant solutions to the problem.

[florianpircher]

Storing the $comments and $status values in local variables does not work, because the values can not be accessed in other snippets (e.g. comments-list can not access variables from comments-form, even if comments-form was included first). Because of that, I am going to use the Kirby registry approach you suggested (with a comments.runtime. prefix).

if (kirby()->get('option', 'comments.runtime.comments') == null) {
  // Create `Comments` object for the current page
  $comments = new Comments($page);
  $status = $comments->process();

  // Store `Comments` object and status for `comments-list` snippet
  kirby()->set('option', 'comments.runtime.comments', $comments);
  kirby()->set('option', 'comments.runtime.status',   $status);
} else {
  $comments = kirby()->get('option', 'comments.runtime.comments');
  $status = kirby()->get('option', 'comments.runtime.status');
}