I would expect that a collection method called join joins two collections into one. Not turning them into a string. I am a bit unsure that this is a common use case but happy to get to know if I'm wrong.

@distantnative

• PHP alts join for implode
• there's A::join already
• Laravel also has one
• there's already add for adding, I could also PR merge as an alternative

I think it makes sense since we already have A::join that does same thing.

Fair. @adamkiss Would you be able to fix the two code style errors?

@distantnative yeah

Sorry for delay but I just wonder why we used empty separator for for collection join but we used comma for arrays?

public static function join(
  array|string $value,
  string $separator = ', '
): string {
  if (is_string($value) === true) {
    return $value;
  }

  return implode($separator, $value);
}

https://github.com/getkirby/kirby/blob/main/src/Toolkit/A.php#L401-L410

@afbora I think you are right and it makes sense to stay consistent there. Probably should also stick to $separator as parameter name. I can fix it directly on the branch.

@afbora @distantnative Because while I understand why you thought joining array items vas imagined with some kind of tags or something, where commas might be preferable, I think major usage of collection (pages, users) join is html output, where no extraneous elements are wanted.

I think it's better to keep it empty, but settable, rather than default to comma and force every html output usage to unset it

@adamkiss I disagree, I think you are coming from a very specific use case in mind. But joining a collection doesn't have to be about HTML output (in fact I think your example use case with the snippet is more an outlier), e.g.

$venues = $events->join(as: fn ($event) => $event->venue());

I think it does make sense to keep A::join() and Collection::join() consistent here.

I don't think it's a big deal. Because both spaces and commas can be used in different use cases. But it would be nice to have consistency.

@distantnative I'm not saying it's impossible to come up with a different use case, but I wholeheartedly think that

1. There are more uses cases where the output is HTML in my opinion, especially given the major use of collection and inheriting classes we work with are Pages
2. it's no harder to add then unset

Thus consistency in the direction of default value is IMO detrimental to the major workflow

@afbora the default I set isn't a space, it's nothing, just putting the outputs together

@adamkiss Agree to disagree here. Consistency has a super high value with a framework tool like Kirby - we can tell how it bites us every time we aren't consistent as it defies user expectations. The Collection class is such a general tool, to me it doesn't make sense to add a narrower use-case of HTML (this is where we will disagree) as default, trading off with consistency.

the default I set isn't a space, it's nothing, just putting the outputs together

What I meant by "space" was the spaceless 🙂

Beyond consistency, if I wanted to join a collection or array to a string, I would think it would need a separator to make it readable. So, unlike you, I personally think users would prefer commas.