I just found out this module could be of use for a specific use case I had if it would allow me to define myself a custom string as separator; so I decided to add it myself, I also included support for wrappers for the separator itself.

Use cases:

  • Special theming techniques can be allowed easily, without having to resort to theming overrides.
  • Token view mode can be used for creating paths if using the "/" separator.

The new formatter is quite a lot like the "comma" formatter, except it gives more customization possibilities.

The patch was created using git diff --no-prefix, to allow it to be used by drush make. It includes a bit of refactoring for the options shared with the comma formatter.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jedihe’s picture

FileSize
306 bytes

I'm also providing a drush makefile to make the patch testing easier. Remember to only leave the .make part of the extension of the file.

core = 7.x
api = 2

projects[drupal][type] = core
projects[admin_menu][subdir] = contrib
projects[devel][subdir] = contrib

projects[textformatter][subdir] = contrib
projects[textformatter][version] = 1.x-dev
projects[textformatter][patch][] = http://drupal.org/files/textformatter_custom_separator_a.patch
damiankloip’s picture

Not sure how a make file is easier for me to test than just a patch posted on here that I can just wget to my machine in one command?

This is also a dupe of #1363032: ☃☃☃☃☃☃ I need snowpeoples ☃☃☃☃☃ (although it isn't immediately evident in the title!). So maybe we can use both to settle on this functionality. I think I prefer atleast one approach in the other patch that doesn't validate the seperator string on the field options form. This leaves it more flexible.

jedihe’s picture

The makefile is superb for testing if used with http://drupal.org/project/quickstart VM (drush quickstart-create). With a single command it returns back a fully installed site, with patched modules, in less than a minute. I prefer this way since I can test always on clean installs, without possible polluted DB. This also helps with repeatability of a test scenario.

I've checked the other patch and there's something I don't like: it doesn't validate the seperator string, but it does escape it silently, which is a very annoying behaviour I've seen previously; it's better to just clearly tell the site builder if something isn't going to be used exactly the way she intends; that's also why I decided to support wrapper-class options for the seperator itself, which gives a lot of control in a safe way.

One way I thought of having a cleaner implementation would be to just mix the comma and the custom seperator theming functions, as the other patch does; but remember that patch doesn't support wrapper-class for the seperator itself, which this one does.

damiankloip’s picture

I don't see how that is easier for me than typing in git apply PATCHNAME.....

I will check the patch out soon.

jedihe’s picture

Well, each one has personal preferences... so I know it's useless to give more explanations :)

I'll be waiting for your review. Since you prefer manually applying the patch, maybe you'll have to use -p0 as argument, since it was created with --no-prefix option; in case you want to have a git apply friendly patch, just tell me.

damiankloip’s picture

Had a quick look at this patch. Some of the field setting keys have been changed. What would people do that are upgrading the module? Wouldn't they just lose all of their current settings if they already had comma lists?

damiankloip’s picture

Status: Needs review » Closed (duplicate)

This is a dupe of #1363032: ☃☃☃☃☃☃ I need snowpeoples ☃☃☃☃☃. I think I will take some parts from this patch maybe and some from the other issue.

damiankloip’s picture

Have just committed patch for this issue. The wrappers around the separator is in. Didn't like the valiadation of the separator string on form submission. This didn't really work from my point of view. Say you wanted a character like '&' for instance, check_plain would convert this to & So I think this is better to check on output.