Download & Extend

Missing HTML Tidy should not always be an error

Project:Beautify
Version:6.x-1.x-dev
Component:Code
Category:task
Priority:normal
Assigned:Garrett Albright
Status:needs work

Issue Summary

The following patch rewrites hook_requirements() to make it an error to not have HTML Tidy installed only if the HTML Tidy beautification mode is selected. Our client was getting annoyed by seeing good ol' "One or more problems were detected" due to this "error" even though we're not using the HTML Tidy method.

Even if this patch is not accepted, I would recommend looking through hook_requirements() and refactoring a bit - for example, note that line 15 checks for $phase === 'install' even though with lines 9-10 we're already inside a switch statement for $phase === 'runtime'.

AttachmentSize
requirements.patch1.82 KB

Comments

#1

Looks good but can you reroll with spaces instead of tabs? Patch looks wonky. Cheers

Also, could you use this syntax:

$requirements['beautify_htmltidy'] =

and just return it at the bottom?

Apart from those thing I would be happy to commit.

#2

I think a wise man once said something profound about beggars and choosers… :P

It never made sense to me to declare a variable if you're just going to return it unmodified immediately after the declaration, but I'll play along.

EDIT: This patch is missing a semicolon… I thought I re-added it and saved before rolling the patch, but I guess not. Unfortunately I can't delete files from comments.

AttachmentSize
requirements2.patch 1.97 KB

#3

Try this one instead.

AttachmentSize
requirements3.patch 1.97 KB

#4

Are you interested in being a co-maintainer? You seem to be showing a lot of interest :)

BTW: Patch looks cool. Thanks a lot for rerolling. I'll wait for your reply on the above before committing.

#5

I might be able to help, though frankly I'm only interested in the minification aspects of this module. Two-thirds of the module seems to be dedicated to making the code look prettier instead of compacting it, and I'm not really interested in that.

Now if the module were split into two, a "make pretty" module and a "make compact" module…

#6

Status:needs review» reviewed & tested by the community

All you need to do is switch the REQUIREMENT_WARNING & REQUIREMENT_ERROR around
Before

<?php
         
'severity' => $phase == 'install' ? REQUIREMENT_WARNING : REQUIREMENT_ERROR,
?>

After
<?php
         
'severity' => $phase == 'install' ? REQUIREMENT_ERROR : REQUIREMENT_WARNING,
?>

#7

Status:reviewed & tested by the community» needs work

Option to turn off the notice would be ideal.

nobody click here