[aur-dev] [PATCH] xhtml validation fixes for login_form.php
--- web/template/login_form.php | 45 +++++++++++++++++++----------------------- 1 files changed, 20 insertions(+), 25 deletions(-) diff --git a/web/template/login_form.php b/web/template/login_form.php index 7fd40fb..b2ed066 100644 --- a/web/template/login_form.php +++ b/web/template/login_form.php @@ -1,28 +1,23 @@ <span id="login_bar"> -<?php -if (isset($_COOKIE["AURSID"])) { - print __("Logged-in as: %h%s%h", - array("<b>", username_from_sid($_COOKIE["AURSID"]), "</b>")); -} -else { - if ($login_error) { - print "<span class='error'>" . $login_error . "</span><br />\n"; - } ?> - <form method='post'> - <?php print __("Username:"); ?> - <input type='text' name='user' size='30' - maxlength="<?php print USERNAME_MAX_LEN; ?>" - value='<?php - if (isset($_POST['user'])) { - print htmlspecialchars($_POST['user'], ENT_QUOTES); - } ?>'> - <?php print __("Password:"); ?> - <input type='password' name='passwd' size='30' - maxlength="<?php print PASSWD_MAX_LEN; ?>"> - <input type='submit' class='button' - value='<?php print __("Login"); ?>'> -</form> -<?php } ?> + <?php + if (isset($_COOKIE["AURSID"])) { + print __("Logged-in as: %h%s%h", array("<b>", username_from_sid($_COOKIE["AURSID"]), "</b>")); + } + else { + if ($login_error) { + print "<span class='error'>" . $login_error . "</span><br />\n"; + } + ?> + <form method="post"> + <label><?php print __("Username:"); ?></label> + <input type="text" name="user" size="30" maxlength="<?php print USERNAME_MAX_LEN; ?>" value="<?php + if (isset($_POST['user'])) { + print htmlspecialchars($_POST['user'], ENT_QUOTES); + } ?>" /> + <label><?php print __("Password:"); ?></label> + <input type="password" name="passwd" size="30" maxlength="<?php print PASSWD_MAX_LEN; ?>" /> + <input type="submit" class="button" value="<?php print __("Login"); ?>" /> + </form> + <?php } ?> </span> - -- 1.4.4.4 -- Michael Klier www: http://www.chimeric.de jabber: chi@jabber.shipdown.de key: http://downloads.chimeric.de/chi.asc key-id: 0x8308F551
There are a couple worthwhile things in here but most of this just looks like messing with the indentation and such. Overall tabs are actually used for indentation, though it's not written anywhere. You should try to write shorter patches that don't mess with everything in the file. It's less for the maintainers to read through and less for you to write. Then we can get more patches through in less time and you can write more. :D Win win!
Loui wrote:
There are a couple worthwhile things in here but most of this just looks like messing with the indentation and such. Overall tabs are actually used for indentation, though it's not written anywhere.
I was actually wondering about that because I haven't found a real refernce and I've seen files with a mix of tabs and whitespace. And you're right, most of it is just messing with the indentation. But I'll use tabs from now on :).
You should try to write shorter patches that don't mess with everything in the file. It's less for the maintainers to read through and less for you to write. Then we can get more patches through in less time and you can write more. :D Win win!
I see your point. In case of the search_pkg_form.php patch, well, this was a complete rewrite/restructuring (preserving it's functionality of course). With the end result in mind it's kinda difficult to do such a rewrite in smaller chunks (and I dreamed about the promised cookie already :D). Anyway, thanks for your reply, I appreaciate your advice and will do my best to conform to these guidelines in the future :). Michael -- Michael Klier
On 6/5/08, Loui <louipc.ist@gmail.com> wrote:
Overall tabs are actually used for indentation, though it's not written anywhere.
I thought there was a policy of spaces at one point. 4 spaces for indentation, blah blah.. similar to (but not quite the same as) the pacman codebase code guidelines. http://www.archlinux.org/pacman/HACKING.html I might have been thinking of a different codebase though (like archweb). *scratches head*
Hi Michael, first of all thanks for contributing to the AUR Development. Imho, you must try to use quotes correctly, i mean simples and doubles, you can find a good tutorial here: http://ve2.php.net/types.string Or you can google 'simple vs doble quotes + php', that will apply to you in many programming languages. By example that line: print "<span class='error'>" . $login_error . "</span><br />\n"; --> bad written Will work, and that should be enough, but no, why doble quotes " then ' and then you concatenate the string, imho that should be something like: print '<span class="error">'.$login_error."</span><br />\n"; // I use double quotes for the \n Or just something like: print "<span class='error'>$login_error</span><br />\n"; and that applies in many senses by examples: // why "? use ' for that case when you are not intending to parse some value is better $foo["var"] = "test"; include "foo/bar.php" print $_SESSION["foo"]; $bar = array("foo","bar"); etc Just a recommendation trying to have clean code, i know that AUR isn't complete clean, but the patches should be, then for next releases will be easier to clean and write code correctly. Just a recommendation, again, thanks for your patches :-) -- Angel Velásquez angvp @ irc.freenode.net Linux Counter: #359909 Arch Linux Trusted User
Angel Velásquez wrote:
Hi Michael, first of all thanks for contributing to the AUR Development.
Hi Angel,
Imho, you must try to use quotes correctly, i mean simples and doubles, you can find a good tutorial here: [1]http://ve2.php.net/types.string Or you can google 'simple vs doble quotes + php', that will apply to you in many programming languages. By example that line: print "<span class='error'>" . $login_error . "</span><br />\n"; --> bad written
thanks for your advice but if you take a closer look at the patch you'll notice that I haven't touched that part of the code other than indenting it. And if you take a look at the bigger patch against the search_pkg_form.php I send earlier you'll see that I quote like you recommend here :) (except that I leave spaces for concatenating as IMHO it makes it easier on the eyes eg. "' . $variable . '" but that's a matter of taste I guess).
// why "? use ' for that case when you are not intending to parse some value is better $foo["var"] = "test"; include "foo/bar.php" print $_SESSION["foo"]; $bar = array("foo","bar");
Agreed again + same thing here, take a look at the search_pkg_form.php earlier, I've fixed all that in it ;) as I prefer single quotes in PHP over double quotes as well (on the other hand for HTML if prefer doubles quotes over single quotes ie: class="foo").
Just a recommendation trying to have clean code, i know that AUR isn't complete clean, but the patches should be, then for next releases will be easier to clean and write code correctly. Just a recommendation, again, thanks for your patches :-)
Thanks you for your feedback :). Michael -- Michael Klier
On Thu, 5 Jun 2008 16:33:59 -0700 eliott <eliott@cactuswax.net> wrote:
On 6/5/08, Loui <louipc.ist@gmail.com> wrote:
Overall tabs are actually used for indentation, though it's not written anywhere.
I thought there was a policy of spaces at one point. 4 spaces for indentation, blah blah.. similar to (but not quite the same as) the pacman codebase code guidelines. http://www.archlinux.org/pacman/HACKING.html
I might have been thinking of a different codebase though (like archweb). *scratches head*
Probably. Yeah most of AUR is tabs with a ts=2 modeline, so it looks like two spaces. Anyways...
participants (4)
-
Angel Velásquez
-
eliott
-
Loui
-
Michael Klier