Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TITLE-Attribute should not wrap #28

Closed
NoNoNo opened this issue Apr 3, 2012 · 3 comments
Closed

TITLE-Attribute should not wrap #28

NoNoNo opened this issue Apr 3, 2012 · 3 comments

Comments

@NoNoNo
Copy link

NoNoNo commented Apr 3, 2012

Even with --wrap-attributes yes the TITLE-attributes should (like the ALT-attribute right now) not be wrapped, because the browsers treat this like a PRE formatted text in the tooltips.

Proposed patch:

diff --git a/src/pprint.c b/src/pprint.c
index 375abb8..0b0c995 100644
--- a/src/pprint.c
+++ b/src/pprint.c
@@ -1158,7 +1158,7 @@ static void PPrintAttribute( TidyDocImpl* doc, uint indent,
     {
         if ( TY_(IsScript)(doc, name) )
             wrappable = cfgBool( doc, TidyWrapScriptlets );
-        else if (!(attrIsCONTENT(attr) || attrIsVALUE(attr) || attrIsALT(attr)) && wrapAttrs )
+        else if (!(attrIsCONTENT(attr) || attrIsVALUE(attr) || attrIsALT(attr) || attrIsTITLE(attr)) && wrapAttrs )
             wrappable = yes;
     }

An unrelated minor optimization: putting the wrapAttrs first will allow faster evaluation of the if-statement, when wrapAttrs == false.

@sideshowbarker
Copy link
Contributor

Have you tested this and does it actually work? If so, can you please let me know exactly what config options you have set? Because even when I use "--wrap-attributes no", it still wraps all attributes. And even if I set "wrappable = no" in line 1163, it still wraps all attributes. And if I put an 'fprintf(stderr, "value: %s\n", attr->value);' at line 1128, I can see that the attribute value has already been wrapped before it was passed to the PPrintAttribute function.

Anyway, I agree that tidy should not be wrapping title attributes. But as far as I can see, making this code change is not going to have any affect. There must be something I'm missing here.

@sideshowbarker
Copy link
Contributor

So after walking through the code, I find ParseValue in the lexer and see that is has munge=yes but that can be overridden y TidyLiteralAttribs, which is set by the --literal-attributes config option. So I guess you must have the option set too. Because if you don't have that set, the lexer code is always going to "munge" attribute values long before the pretty-printing code is executed, regardless of whatever options you have set.

But now I find that if I do "tidy --literal-attributes yes --wrap-attributes yes", no attribute values get wrapped, ever. So it's still not clear to me what's going on here.

So It would be really helpful to know exactly which options you have set.

@sideshowbarker
Copy link
Contributor

OK, looking at the docs finally I understand the logic and what --wrap-attributes does. It only wraps attribute values in cases where the value makes the line length longer than the value of the "wrap" option. So anyway, I agree it should not be doing that for the value of the title attribute, so I'll go ahead and make this change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants