I suggested an alternative to comment_upload at #739690-16: Compatibility with comment_upload
and provided more details at #739690-28: Compatibility with comment_upload
but it turns out that there are some points how would worth to discuss
so I'm creating this separated issue to deliberate about them
(this is not a "how to" neither a FAQ regarding how to achieve it, rather a place to settle up the best approach to follow)

Comments

arhak’s picture

as Takafumi pointed at #739690-34: Compatibility with comment_upload
http://drupal.org/files/issues/inspect.png
the difference is a fact and it is properly detected
but nothing is rendered

the point is that comment_driven aims to display difference introduced by a comment to driven properties
if the change is introduced in other fashion it won't take care of it
and if the rendered revisions don't differ, well, it is expected that it will render the visual difference

selecting the file to not be listed tells filefield to not render it
therefore when comment_driven tells CCK to render the before/after diff it turns out that only empty string are rendered

it would be simple to treat the case apart and render it as it would be listed (perfectly achievable)
but the question is, how many ways different kinds of fields can be rendered and how many of them would be empty string?
for instance, having no access to a page makes menu links get hidden,
what if a field does something like it?

I think that comment_driven shouldn't take care of how fields render themself
and not discriminate to take consideration with filefield
most of all, when there is an option which makes impossible to the user to unselect the "list" option #739690-33: Compatibility with comment_upload
and if it is desired that the attachments don't show up on the node there is Node Edition Policy which allows to hide them

EDIT: typos

arhak’s picture

for instance, create a text field with format
make it a driven property and write some text into it with format "Filtered HTML"
then through a comment (with cdriven) change the input format for that field, but leave the exactly the same text,
how could comment_driven know that the change doesn't reflects visually?
should it take care of comparing the resulting rendered value?
what if you write a text with "Full HTML" input format and later change it to "Filtered HTML" and the resulting text is empty?
should it take care of it as well?
I think that is not our business
that would be the diff module approach, which is totally different

Takafumi’s picture

I see what you mean, but it seems the behavior is not good when empty strings are rendered, because it looks like a bug.
So, how about hiding "xxx: »" if strings are empty?

arhak’s picture

that would shift the "apparent bug" to other scenarios
if an actual change existed the "Allow empty comment" rule could be used (if selected in config)
if no diff_summary is shown, what would justify an absolutely empty comment being allowed?

OTOH, suppose a field which render something like:
<div style="display:none">foo-bar-baz...</div> (or any class hidden by CSS/JS)
it wouldn't be detected as an empty string, and though it wouldn't render anything visible
what would be considered an empty string then?

there are several reasons to not get involved on how data is rendered
basically I can't properly know/interpret what would the user see/perceive
therefore, addressing some punctual cases would make remaining cases more odd/awkward

I think the site-builder must know what he/she is dealing with
for instance, suppose a vocabulary with a duplicated "Term1", one with tid=1 and the other one with tid=2
lets say that vocabulary uses a select dropdown, and therefore, there is an actual difference when the user selects tid=1 or tid=2, and what would be the rendered diff?
VocName: Term1 » Term1
similar problem, and not with empty strings

also note, that in a case like this comment_cck wouldn't render any diff, since they compare resulting rendered values, and comment_driven aimed not to do so
but really, can't tell how to better display a diff, without taking an approach closer to what diff module does
which I consider undesirable, since the diff rendered would be more related to how internal data is represented, and also a per-line comparison would be needed, because, for instance, a multi-line input being rendered inline would remain the same if just the line-breaks are moved (or the body field regarding its <!-- break --> mark)

arhak’s picture

Title: Discussion: alternative to comment_upload » Discussion: rendering existing differences that doesn't display/look like differences at all

note that I'm not reluctant to a change around these issues
as long as a consistent proposal comes out from this discussion
but the best I can come up right now would be a pluggable diff_render post-processor with configurable options
starting with kind of "enforce rendering filefield as listed even when they weren't" (with a more appropriate name, of course)
that way, not enabling that sub-mod would bring the original conception
while having it enabled would bring more bizarre behavior for those unconsidered cases, but it would be an open door to feature/support requests without burdening the core of comment_driven with inconsistent behaviors

even, such a sub-mod could have all those "alternative renderers" activated by default
that way it would be just a matter of enabling it
but I think that requests could never stop, since there are hundreds of fields out there, and therefore, several ways of having "imperceptible" differences

PS: a related pending issue in the #711094: Roadmap would be rendering some kind of mark whenever there is a property or value gone

Takafumi’s picture

that would shift the "apparent bug" to other scenarios
if an actual change existed the "Allow empty comment" rule could be used (if selected in config)
if no diff_summary is shown, what would justify an absolutely empty comment being allowed?

You're exactly right. I've not found the case which you described.