Pass by reference issue

Gotta say again - the install-feature setup works really well!

This test was on an OSX, PHP 5.2.14 - slightly home-compiled version of PHP. (where pass-by-reference is a nasty warning notice, but not full death. on 5.3 it's fatal)
With all warnings (error_reporting = E_ALL & ~E_NOTICE) printed to screen. display_errors = On

All Drupal versions as specified in the makefile.

I saw the jquery_ui lib come down the pipe when 'make' ran. Just not sure where drush stored it afterwards - can't find it now anyway. I used to run a tweaked version of drush (getting it to play nice with other makefiles) but today I'd replaced it with the freshest git version, so no surprises there?

Yes, you are quite right. if I revert to a clean distro, the UI buttons do behave like they were supposed to! !!
However, I had to turn of all error reporting on my dev machine to stop the warnings. The warnings to the screen made form submission and AJAX entirely fail.

I've had an extensive peek under the hood so far, and (when I figured it out) I think the re-use of the field subforms that happens - hiding them then revealing them in the dialog) is pretty sweet.

However, I have opinions about yet another markdown syntax and yet another pseudo tag.
I'll attach my roadmap/rant on that here, just as an opinion piece. I've been planning to share it somewhere, so it may as well be here.

In short - Why invent a tag called
<wysiwyg_field id="wysiwyg_fields-field_image-0-custom_formatters_formatter_custom_formatters_wf_img_caption" class="wysiwyg_fields wysiwyg_fields-field_image"></wysiwyg_field> when that's just non-validating html that needs to be converted out of each time you switch edit mode?
Why invent another markdown [wysiwyg_fields-field_image-1-custom_formatters_formatter_custom_formatters_wf_img_caption]
that you have to keep swapping in-and-out of? That markdown is not going to be managed by a human editor, so why 'simplify' it?
And, y'know ... a bit ugly, right?

I think (and this is just IMO of course) a syntax such as:

<div class="wysiwyg_fields wysiwyg_fields-field_image" wf:element="field_image" wf:index="0" wf:formatter="wf_img_caption"   ></div>

... can do everything you are doing, and bring along a whole bunch of advantages too.

Of course, I'm not suggesting a full re-think from your end, just sharing the direction that I'm planning to go with a thing like this (... hm, can no longer link to my cvs sandbox .. )
Anyway, I'll attach my roadmap, perhaps it contains an idea.

OK, that looks like an answer.
With error-messages hushed, and your modifications above applied, things continued to work.
With error messages turned on again, I got a repeat of the first two warnings from includes/system.inc:39, includes/system.inc:46

so I re-patched those :-)

Yes, I think we have it all working. No more warnings, fully working buttons!
I think that's an ok fix. not beautiful, but gritty stuff like that isn't.

FYI, about repeating it, one of the verbose messages also gave me this hint:

Warning: Call-time pass-by-reference has been deprecated; If you would like to pass it by reference, modify the declaration of drupal_alter(). If you would like to enable call-time pass-by-reference, you can set allow_call_time_pass_reference to true in your INI file in /Library/WebServer/Documents/wysiwyg_fields_test/sites/all/modules/contrib/wysiwyg_fields/includes/system.inc on line 39

I didn't plan on setting 'allow_call_time_pass_reference' for my test box, but that may be your state.

I know what you mean with the block vs inline element choice. This is an issue I've looked at already.
I'm still expecting there will be a way to use existing semantic tags + css to behave in all the ways we need it to. The actual tag type doesn't even matter to me as I'd just be looking for <* class='wysiwyg_field'>. So a span is fine. I'd choose a specially styled span over a new 'tag' any day.

Fighting with the various editor libraries, I can sympathize with. But aren't any of them, or any xhtml-compliant browsers within their rights to just entirely discard or ignore non-validating, non-namespaced elements they find in the DOM? If the editor declares a DOCTYPE, it should be enforcing it. The fact that they currently don't seem to be strict on it seems more by luck than design. I mean, it's great that you've found that it works, but I don't trust that it should work into the future.
If you namespaced it, I'd relax a lot more.

if the referenced CCK field is modified by an external source

Yeah fully.
I'm totally keeping on with the filter approach. Yes I think that any of these remote elements can be dynamically replaced with filters. My version does the same as yours in that way
- except that my filter scans for the semantic tags and does the replace, not for the custom tokens.
What gets saved in the database is actual HTML, not pseudo-code. I like it that way.
That's largely because I'm comfortable with XML and XPATH now, and (when parsing XHTML) prefer it over regular expressions.
So just getting an element with a class is trivial - though it can be a nightmare in regular expressions.
... yes, I do my hook_filter with xpath and DOM manipulation. :-B

Well, I don't have the full answer, and your discussion points are helpful in trying to highlight any weaknesses with the plan.

Yes, We always need a wrapper of some sort. Hell no to comment tags.
I'm just expecting that a div with display:inline-block is going to be mostly acceptable.

Given Drupals div-itis, encountering <img/><div>Text</div> without a wrapper div is pretty unlikely, and not a common layout thing. Why would you do that and not put a container of some sort around it? With display:inline you get whatever was needed in the layout, plus a class tag to leverage. I consider a case where you absolutely have to insert just HTML fragments without a wrapper where display:inline; won't do - to be an edge case I don't think needs supporting.

The other downside is that technically someone would be able to write the current format by hand, allowing those not using a Wysiwyg input filter to still utilize the module. Of course they could do that with your format as well, but it could be harder to explain the syntax...

Harder? I gotta disagree. One of the reasons I'm going down my track is because valid attributes make sense for anyone who is dealing with HTML, or has ever seen HTML source. There is 30 years of background to that syntax.

I can't agree that (for someone who is already in HTML code-view) that this would be easier to type, maintain and understand

[wysiwyg_fields-field_image-1-custom_formatters_formatter_custom_formatters_wf_img_caption]

... than this:

<div class="wysiwyg_fields wysiwyg_fields-field_image" wf:element="field_image" wf:index="0" wf:formatter="wf_img_caption"   ></div>

Looking at the other 20 'insert' modules that have injected pseudo-tags, they have all had to invent their own syntax. Each unique, each limited, each requiring the user to learn different syntax rules. Initially because it was 'simpler' to write [img:1]. But then they had to break, twist and extend the syntax until it was something that was so close to SGML that you wonder why they couldn't see the similarity [img_assist|nid=2|title=My title|desc=My description|align=right|width=200|height=150|link=url,http://www.google.com]

extensibility is a big one. Can you cover every possible argument in the ID token? I'm looking immediately at adding an optional 'cachelifetime' attribute to my embeds to indicate their volatility.
I like the idea that maybe it will make sense to apply styles to the item - width:45%, float:right, margin-left:1em.
By fixing on a single fixed string-based identifier so early, you may be implicitly saying "I will never add any more features to this model". I don't think that is true.

I'm looking at more than just field-embeds here, in my roadmap. If I were to do this with a views_embed type thing ... then I need space in the syntax to add arg1="2011". Not relevant to where you are going with wysiwyg_fields this week but I see a pattern there.

My preference for real markup in the real markup place may seem to be an internal issue only, but I've spent many of my dev years doing migrations in and out of HTML systems and dozens of different CMSs. The more unique tricks your 'text' has in it, the more reliant its 'content' is on the display mechanism - the less portable, the less future-proof it is. It becomes less internally consistent, less self-evident.
Just last Christmas, we had to migrate a Drupal 5 site to Drupal 6. Great, only it was using one of the many flavors of pseudo-tag, just to embed images. It turned out to be a deprecated one. This was thousands of pages of legacy content, using an input filter that wasn't available in D6.
Given that we were replacing the functionality with a (slightly) better version of the same thing, life would have been so much better if the [img|attachment:1] tag was just an <img src="..." class="img-attachment-1"> or something like that.
By looking behind, I can think ahead.

And maybe the main reason that I want to take this approach with a wysiwyg plugin - is I can avoid that 3/4 of the javascript code that does nothing except swap between various markup formats - internal token, wysiwyg-render, textview-markdown, actual-render every time you click a button. If we could stop rewriting that card-trick for each editor and concentrate on actual features I think the code would be sweeter to write.

Anyway ... all just brain-dumps. I'm not talking you into changing what currently works fine for you, just sharing where I am looking towards.

I would be ever so grateful if you could provide a patch as it sounds as if you've already dealt with the regular expression

I avoid the regular expression altogether! If you are limited to regexp, then the simple tokens have a huge head-start. And probably expalin why the earlier efforts went that way.
But ... I use XPATH and jquery. So the opposite is true for me!
When we HAVE jquery, forget string-splitting :-)

I don't have a patch for your module yet - but I did have a proof-of-concept of my annotated divs getting plugged into WYSIWYG+CKEditor in my CVS sandbox. Not sure how to get it out now. I'll have to sit down with git for a little while first I think.

Thanks for sharing re WYSIWYG editors vs paragraph tags. I'd not seen that. Yep, that's a poser worth looking at.
It again raises the question of what wysiwygs that helpfully 'correct' our block wrapping would do or not do with the fake tag.. Luckily it seems to be nothing?

So whatever... I said I don't care about what the element is. Maybe we can use div,span, whatever as needed? A per-formatter option. As I said, using xpath I'd just be looking for <* class='wysiwyg_field'>
It adds a new layer of settings ... true. The issue/challenge quite valid. But - but because we are building on an extensible base, issues can be overcome - I think.
... I think being able to define your wrapper element will pay off in HTML5 <aside>, <figure>, <dialog> . Having to support this option could be an early blessing!
IMO, the 'wrapper' can also be the element itself - where appropriate (excluding only your ungrouped img+text example). If a formatter does nothing but insert an image, we can go
<img src="..." class="wysiwyg_fields" wf:element="field_image" wf:index="0" wf:formatter="wf_img_100x100" />
... and still be round-trip compatible.
... This is new thoughts, I've not considered the ramifications of that fully-integrated approach. So it probably has some holes to pick also.

YES, looking for an element that is just a semantic wrapper without block or inline would be nice. I agree with your thoughts on that. Can't see it in HTML5 either.

I did see the element wrapper tag name getting set in the code, I tried tweaking it back to div to see what would break, but it seems there were a number of assumptions about the tagname elsewhere ... or I just tweaked the wrong place, it didn't have the quick result I was looking for.

Yeah, you are right that divs aren't technically allowed within Ps. It's nice to see the editor enforcing the rules there - I guess!
I'm not wanting to open up an HTML5 discussion, just throwing it out there as a progressing point to consider for selecting tags - or, to be precise, as a point against selecting any one tag.

I have big chunks of code in my sandbox.

An early imageembedder module that annotates a normal wysiwyg-safe img tag with a bunch of metadata that becomes directives to the render filter.

 * IF a found image (using normal <img src=""> tags is found to have an entry in
 * the files table as either a thumbnail or preview of an image node, then the
 * html will be extended with the extra HTML and CSS.
 *
 * IF the found image is found to have a specially named class assigned (easy
 * enough to do through the WYSIWYG or similar client-side tool) then optional
 * extra processes are performed.
 *
 * Currently supported: if the img has a class matching 'imagecache-presetname'
 * then the PRESET is rendered instead of the named image.
 *
 * EG :<img src="/sites/default/files/images/huge-screenshot.jpg" class="
 * imagecache-cropped_screenshot" >
 *
 * Becomes
 * <img src="/sites/default/imagecache/cropped_screenshot/files/images/huge-
 * screenshot. jpg" class=" imagecache-cropped_screenshot" >

Additional render options (triggered by the class on the image) do things like turn the alt tag into a caption and wrap it in a floated div.

It includes an XPATH-driver Drupal filter, and proved that worked to me. I use it here among other places.
So, round-trip pure-xhtml as semantic tags worked for me.

I can send through the hook_filter - XPath support function (it made me feel happy), but I need to push some of my sandbox public still.

This year, I'm working on ... pretty much what I've described so far. Though not local field-based like yours at all, it's currently embedding - (other nodes as full or teaser, blocks, views) using the syntax I prefer. So far I link straight to a node, setting parameters on the embed object. You set up a nodereference and embed that. I like your method better :-)
It exists, it's taken all the ideas I could collect from me README above. It's in sandbox, and last week I could have linked you to it. I just gotta get it out through git now. And, as usual, I'd like to tidy it up before then. But further work on it will be sponsored at my end - but I've been told to pause until the quote clears.

I've now learned how to write wysiwyg-compatible plugins, and as soon as I can figure out how to trigger a right-click context menu on my elements ... I'l be very happy.

Right.
http://drupal.org/sandbox/dman/1075470
is the sandbox for my proof-of-concept. Not ready to release on the world at all.

It includes embedder_filter() which runs embedder_dom_process() which is where my xpath happens, with the assistance of a few helper funcs.
Yeah, it uses 40 lines to to what regexp can probably do in 6 lines.
Partly that's because it's stepped out for my own debugging, partly because DOM can be quite cumbersome.
Yeah performance is sure to be several times slower that string replacements.
Yeah it's PHP5 only.

But it's a direction I'm exploring, and haven't found a reason to stop yet.

I really don't want to derail you with my ideas, so I think you'd be justified in holding off on the token format changes in the main trunk. Though .. isn't git supposed to support us trying out significant branches now? I have things to learn about that..

but ImageField via a Node Reference is so much better.

We are on a wavelength then!! Images as nodes forever.

PS, if you haven't already, check out http://drupal.org/project/nrembrowser - it does some groovy ideas with nodereference and linking to directly media within the referenced items