Hi,
I really like this module, it does exactly what I needed when re-writing a non-drupal site to drupal and wanting not to lose visitors from all the bookmarked and/or cached old page links. Thanks very much.

However, I did have a little trouble with entering the custom redirect pairs in the textarea field. The site has a wysiwyg page editor and this was converting the text into html by adding <p> tags to each line, which did not help the matching. If When I turned to plain text editor all the pairs were then joined into one single line. Also initially I only entered one custom redirect, and there is a small bug in the code which means that unless you enter a carriage-return at the end of the single line then no values are processed. I got to thinking about how to make the text entry easier, so I thought I would re-write the input form to have pairs of text fields instead of one large text area. All the values are displayed and five blank rows are added for new entries after each save. Another benefit of this is that the processing of the pairs is simpler because they are stored in a structured array so we do not need to explode the text string manually. Validation is also added to make sure that complete pairs are entered.

Attached is a screen shot of the new admin fieldset and a patch will follow. Hopefully you will find this useful and would consider committing it.

Thanks
Jonathan

CommentFileSizeAuthor
#3 _878954.customerror.convert_to_separate_text_fields_2.d6.patch8.17 KBjonathan1055
#1 _878954.customerror.convert_to_separate_text_fields.d6.patch8.91 KBjonathan1055
customerror admin setting.jpg407.37 KBjonathan1055

Comments

jonathan1055’s picture

jonathan1055 commented
Status: Active » Needs review
StatusFileSize
new _878954.customerror.convert_to_separate_text_fields.d6.patch8.91 KB

Here is the patch, against the dev of 10th August.

kbahey’s picture

kbahey commented
Status: Needs review » Needs work

Patches that add a lot of code are a problem: they add complexity and make it difficult to maintain software in the long run.

This patch is too big and too complex. Can you simplify it significantly?

jonathan1055’s picture

jonathan1055 commented
Status: Needs work » Needs review
StatusFileSize
new _878954.customerror.convert_to_separate_text_fields_2.d6.patch8.17 KB

Sorry, the patch above was not against todays dev. Here is a correct one.

jonathan1055’s picture

jonathan1055 commented

Hi,
I didn't see your response before I re-posted the patch in #3 (the internet equivalent of 'crossed in the post'), and this also unintentionally set the status back to 'needs review'.

I know that patches which look complex can be bad, but not in all cases. The underlying code change is quite simple. Once the patch is committed I do not agree that it is harder to maintain.

Simplifying it ... well, not sure if I can, because it needs to do only what it does do. I am happy to explain it in more detail. It only addresses the one issue I've talked about, and most of the code is just adding the bits for the better form input. If you use a file-compare utility there are only 9 chunks of code change, and these are confined to changes in three hooks and the addition of three new hooks. It certainly really helped me on the site I was handing over. If there is a chance that you may be interested I'll put a full explanation here, just let me know.

Jonathan

gisle’s picture

gisle
he/him
Primary language Norwegian Bokmål
Location Norway
commented
Status: Needs review » Closed (won't fix)

This patch is too complicated and nobody has bothered reviewing it is three years. It won't be committed. Time to close.

jonathan1055’s picture

jonathan1055 commented

OK fair enough. I still like the idea, and it has helped me a great deal on my sites. But no one else seemed to be interested, so as you say - time to close.
Jonathan