© 2004
Canonical Ltd.
•
Terms of use
•
Data privacy
•
Contact Launchpad Support
•
Blog
•
Careers
•
System status
•
bbfa235
(Get the code!)
Great patch, I really like it. This options page badly needed some love. I love the live syntax check you implemented :D
But there are some things which should be improved before we merge this:
* It is now possible to introduce line breaks in the filename. You tried to work around this in Script.py, but that does not work reliable as you miss newlines inside of parse_text. But I wouldn't implement this there anyway as newlines might as well be needed in the script if you apply the script to tags and not to the filename. Instead I would strip all newlines from the naming string prior to invoking the Parser in File#_script_
* The two methods test and va_test in RenamingOptionsPage are nearly identical. I would source out all the common code to a helper method.
* Fix the copyright for the files you added :)
I have a few minor fine-tuning issues regarding the UI:
* I find it confusing that the script error message for the va format shows up above the textbox for the normal format. Maybe show it above the lower textbox
* If you disable the renaming or moving of the files the corresponding input elements used to grey out. This is no longer the case, but this would make it more clear how these checkboxes influence the settings.
* Also I find the checkboxes for file moving/renaming hard to find at the upper right corner. But it looks clean and I have no better idea :)
* You have introduced a new option page layout with large headlines. While I like it I think this should stay consistent. I suggest changing all option pages to include such a headline, repeating the option title from the left pane.
* I would name the option headlines the same as the titles in the left pane (both "Rename files" and not "File Renaming" and "Rename files".
That review got longer then I expected. Thanks for the patch. I'm looking forward to have that into Picard :) If you have questions regarding my comments please contact me, best would be using the mb-devel mailing list.