[aur-dev] Adding the HE language.

newer
[aur-dev] [PATCH] "logged before"...

older
[aur-dev] Package blacklist for...

Netanel Shine

7 Aug 2010 7 Aug '10

2:16 p.m.

Hey there, i was working on translating the AUR to my language, Hebrew. I am adding to this message the gzipped patched file. btw. i know his name lame, but i think he will work just fine. THanks! -- Netanel Shine

Attachments:

0001-modified-AUTHORS.patch.tar.gz (application/x-gzip — 5.4 KB)

Show replies by date

Loui Chang

7 Aug 7 Aug

9:23 p.m.

On Sat 07 Aug 2010 17:16 +0300, Netanel Shine wrote:

...

Hey there, i was working on translating the AUR to my language, Hebrew.

I am adding to this message the gzipped patched file.

btw. i know his name lame, but i think he will work just fine.

Ah yeah something went wrong with your commit message. Just glancing at the translation real quick I notice one error

...

+$_t["Confirm your e-mail address:"] = "netanelshine@gmail.com";

That message should be translated rather than putting your actual email. If you're using the translation tool, there are a lot of strings that it won't pick up as well (it's buggy), so test your translation with your running copy of the AUR to see if you missed anything. Cheers and thanks for your contributions.

Netanel Shine

9:27 p.m.

Thanks a lot :) fixing it now, something like an hour and i will send just the he.po again On Sun, Aug 8, 2010 at 12:23 AM, Loui Chang wrote:

...

On Sat 07 Aug 2010 17:16 +0300, Netanel Shine wrote:

...
Hey there, i was working on translating the AUR to my language, Hebrew.

I am adding to this message the gzipped patched file.

btw. i know his name lame, but i think he will work just fine.

Ah yeah something went wrong with your commit message. Just glancing at the translation real quick I notice one error

...
+$_t["Confirm your e-mail address:"] = "netanelshine@gmail.com";

That message should be translated rather than putting your actual email. If you're using the translation tool, there are a lot of strings that it won't pick up as well (it's buggy), so test your translation with your running copy of the AUR to see if you missed anything.

Cheers and thanks for your contributions.

-- Netanel Shine

Netanel Shine

10:05 p.m.

he.po File, Fixed: On Sun, Aug 8, 2010 at 12:27 AM, Netanel Shine wrote:

...

Thanks a lot :) fixing it now, something like an hour and i will send just the he.po again

On Sun, Aug 8, 2010 at 12:23 AM, Loui Chang wrote:

...
On Sat 07 Aug 2010 17:16 +0300, Netanel Shine wrote:

...
Hey there, i was working on translating the AUR to my language, Hebrew.

I am adding to this message the gzipped patched file.

btw. i know his name lame, but i think he will work just fine.

Ah yeah something went wrong with your commit message. Just glancing at the translation real quick I notice one error

...
+$_t["Confirm your e-mail address:"] = "netanelshine@gmail.com";

That message should be translated rather than putting your actual email. If you're using the translation tool, there are a lot of strings that it won't pick up as well (it's buggy), so test your translation with your running copy of the AUR to see if you missed anything.

Cheers and thanks for your contributions.

-- Netanel Shine

-- Netanel Shine

Netanel Shine

10:17 p.m.

btw, it should be RTL, if you didnt know, mybe because of that it display like a bug. On Sun, Aug 8, 2010 at 1:05 AM, Netanel Shine wrote:

...

he.po File, Fixed:

On Sun, Aug 8, 2010 at 12:27 AM, Netanel Shine wrote:

...
Thanks a lot :) fixing it now, something like an hour and i will send just the he.po again

On Sun, Aug 8, 2010 at 12:23 AM, Loui Chang wrote:

...
On Sat 07 Aug 2010 17:16 +0300, Netanel Shine wrote:

...
Hey there, i was working on translating the AUR to my language, Hebrew.

I am adding to this message the gzipped patched file.

btw. i know his name lame, but i think he will work just fine.

Ah yeah something went wrong with your commit message. Just glancing at the translation real quick I notice one error

...
+$_t["Confirm your e-mail address:"] = "netanelshine@gmail.com";

That message should be translated rather than putting your actual email. If you're using the translation tool, there are a lot of strings that it won't pick up as well (it's buggy), so test your translation with your running copy of the AUR to see if you missed anything.

Cheers and thanks for your contributions.

-- Netanel Shine

-- Netanel Shine

-- Netanel Shine

Loui Chang

8 Aug 8 Aug

1:30 a.m.

On Sun 08 Aug 2010 01:17 +0300, Netanel Shine wrote:

...

btw, it should be RTL, if you didnt know, mybe because of that it display like a bug.

Sorry I don't really know what Hebrew is supposed to look like. Is there a problem displaying it properly? http://louipc.mine.nu/images/he.po.jpg http://aur.louipc.mine.nu/index.php?setlang=he Looks like you did miss some strings. Do you want to add them and then submit a new patch?

Netanel Shine

3:21 a.m.

For the non-translate strings ill send you a patch, but we have another problem, its shows ok, but it should be RTL not LTR. can you do it some way that if use will choose hebrew the page will become to be RTL? On Sun, Aug 8, 2010 at 4:30 AM, Loui Chang wrote:

...

On Sun 08 Aug 2010 01:17 +0300, Netanel Shine wrote:

...
btw, it should be RTL, if you didnt know, mybe because of that it display like a bug.

Sorry I don't really know what Hebrew is supposed to look like. Is there a problem displaying it properly?

http://louipc.mine.nu/images/he.po.jpg http://aur.louipc.mine.nu/index.php?setlang=he

Looks like you did miss some strings. Do you want to add them and then submit a new patch?

-- Netanel Shine

Loui Chang

4:33 p.m.

On Sun 08 Aug 2010 06:21 +0300, Netanel Shine wrote:

...

For the non-translate strings ill send you a patch, but we have another problem, its shows ok, but it should be RTL not LTR. can you do it some way that if use will choose hebrew the page will become to be RTL?

I would appreciate your help with this. I'm not really sure what the issues are.

Ng Oon-Ee

4:47 p.m.

On Sun, 2010-08-08 at 12:33 -0400, Loui Chang wrote:

...

On Sun 08 Aug 2010 06:21 +0300, Netanel Shine wrote:

...
For the non-translate strings ill send you a patch, but we have another problem, its shows ok, but it should be RTL not LTR. can you do it some way that if use will choose hebrew the page will become to be RTL?

I would appreciate your help with this. I'm not really sure what the issues are.

Maybe I'll interject at this point. The issue with Hebrew (as with some other Arabic languages) is that the words are read right-to-left. I would assume the translations submitted by Netanel Shine need to be 'read' right-to-left as well. I'm not sure how the AUR does things (how the translation units are) but would a suitable 'hack' be for the translations themselves to be in reverse order? So for example the English A B C would be translated as the Hebrew c b a?

Netanel Shine

5:08 p.m.

Not exactly. you can see in the israeli archlinux communiuty : http://archlinux.org.ilhow the page is RTL (not like the main site of arch.) for now you could deploy to translation (yep, meantime in LTR) but we should all work for working RTL Support. The israeli community is the first archlinux community which based on RTL language. 2010/8/8 Ng Oon-Ee

...

On Sun, 2010-08-08 at 12:33 -0400, Loui Chang wrote:

...
On Sun 08 Aug 2010 06:21 +0300, Netanel Shine wrote:

...
For the non-translate strings ill send you a patch, but we have another problem, its shows ok, but it should be RTL not LTR. can you do it some way that if use will choose hebrew the page will become to be RTL?

I would appreciate your help with this. I'm not really sure what the issues are.

Maybe I'll interject at this point. The issue with Hebrew (as with some other Arabic languages) is that the words are read right-to-left. I would assume the translations submitted by Netanel Shine need to be 'read' right-to-left as well.

I'm not sure how the AUR does things (how the translation units are) but would a suitable 'hack' be for the translations themselves to be in reverse order? So for example the English A B C would be translated as the Hebrew c b a?

-- Netanel Shine

PyroPeter

5:39 p.m.

On 08/08/2010 07:08 PM, Netanel Shine wrote:

...

Not exactly.

you can see in the israeli archlinux communiuty : http://archlinux.org.ilhow the page is RTL (not like the main site of arch.) for now you could deploy to translation (yep, meantime in LTR) but we should all work for working RTL Support.

The israeli community is the first archlinux community which based on RTL language.

2010/8/8 Ng Oon-Ee

...
On Sun, 2010-08-08 at 12:33 -0400, Loui Chang wrote:

...
On Sun 08 Aug 2010 06:21 +0300, Netanel Shine wrote:

...
For the non-translate strings ill send you a patch, but we have another problem, its shows ok, but it should be RTL not LTR. can you do it some way that if use will choose hebrew the page will become to be RTL? I would appreciate your help with this. I'm not really sure what the issues are.

Maybe I'll interject at this point. The issue with Hebrew (as with some other Arabic languages) is that the words are read right-to-left. I would assume the translations submitted by Netanel Shine need to be 'read' right-to-left as well.

I'm not sure how the AUR does things (how the translation units are) but would a suitable 'hack' be for the translations themselves to be in reverse order? So for example the English A B C would be translated as the Hebrew c b a?

It seems like it would be a good idea to add direction:rtl; to the CSS. This makes text right-aligned and cuts off text at the right side. -- irc: PyroPeter at freenode

Netanel Shine

5:43 p.m.

Good idea, i hope someone will do that ;) (and also will deploy and HE lang) On Sun, Aug 8, 2010 at 8:39 PM, PyroPeter wrote:

...

On 08/08/2010 07:08 PM, Netanel Shine wrote:

...
Not exactly.

you can see in the israeli archlinux communiuty : http://archlinux.org.ilhow the page is RTL (not like the main site of

arch.) for now you could deploy to translation (yep, meantime in LTR) but we should all work for working RTL Support.

The israeli community is the first archlinux community which based on RTL language.

2010/8/8 Ng Oon-Ee

On Sun, 2010-08-08 at 12:33 -0400, Loui Chang wrote:

...
...
On Sun 08 Aug 2010 06:21 +0300, Netanel Shine wrote:

...
For the non-translate strings ill send you a patch, but we have another problem, its shows ok, but it should be RTL not LTR. can you do it some

way

...
that if use will choose hebrew the page will become to be RTL?

...
I would appreciate your help with this. I'm not really sure what the issues are.

Maybe I'll interject at this point. The issue with Hebrew (as with some other Arabic languages) is that the words are read right-to-left. I would assume the translations submitted by Netanel Shine need to be 'read' right-to-left as well.

I'm not sure how the AUR does things (how the translation units are) but would a suitable 'hack' be for the translations themselves to be in reverse order? So for example the English A B C would be translated as the Hebrew c b a?

It seems like it would be a good idea to add direction:rtl; to the CSS. This makes text right-aligned and cuts off text at the right side.

-- irc: PyroPeter at freenode

-- Netanel Shine

Loui Chang

9:53 p.m.

On Sun 08 Aug 2010 20:43 +0300, Netanel Shine wrote:

...

Good idea, i hope someone will do that ;) (and also will deploy and HE lang)

On Sun, Aug 8, 2010 at 8:39 PM, PyroPeter wrote:

...
It seems like it would be a good idea to add direction:rtl; to the CSS. This makes text right-aligned and cuts off text at the right side.

Please avoid top-posting. I really don't know how to go about providing such a feature, but contributions are definitely welcome.

Netanel Shine

10:01 p.m.

you can provide for now the HE support? (even its temp without RTL?) On Mon, Aug 9, 2010 at 12:53 AM, Loui Chang wrote:

...

On Sun 08 Aug 2010 20:43 +0300, Netanel Shine wrote:

...
Good idea, i hope someone will do that ;) (and also will deploy and HE lang)

On Sun, Aug 8, 2010 at 8:39 PM, PyroPeter wrote:

...
It seems like it would be a good idea to add direction:rtl; to the CSS. This makes text right-aligned and cuts off text at the right side.

Please avoid top-posting.

I really don't know how to go about providing such a feature, but contributions are definitely welcome.

-- Netanel Shine

Loui Chang

10 Aug 10 Aug

1:27 a.m.

On Mon 09 Aug 2010 01:01 +0300, Netanel Shine wrote:

...

you can provide for now the HE support? (even its temp without RTL?)

Please avoid top-posting. Thanks. It will be provided with the next update of the AUR. I'm planning on doing this soonish. For now, it has been pushed to the git repo.

foo bar

14 Aug 14 Aug

12:28 p.m.

On 08/08/2010 12:17 AM, Netanel Shine wrote:

...

btw, it should be RTL, if you didnt know, mybe because of that it display like a bug.

If we set the lang-attribute on the <body>-tag, we can use CSS to swich the whole body to direction:rtl;. (Somehow my chromium does not check the <xml>-tag's lang-attribute.) This is demonstrated here: http://keks.selfip.org/tmp/aur-lang.html I will upload a patch for that shortly. http://keks.selfip.org/tmp/aur-lang.html

PyroPeter

1:24 p.m.

On 08/14/2010 02:28 PM, foo bar wrote:

...

On 08/08/2010 12:17 AM, Netanel Shine wrote:

...
btw, it should be RTL, if you didnt know, mybe because of that it display like a bug.

If we set the lang-attribute on the <body>-tag, we can use CSS to swich the whole body to direction:rtl;. (Somehow my chromium does not check the <xml>-tag's lang-attribute.) This is demonstrated here: http://keks.selfip.org/tmp/aur-lang.html

I will upload a patch for that shortly.

http://keks.selfip.org/tmp/aur-lang.html

I just noticed you are setting xml:lang and lang on the <html>-tag. So there isn't even any change needed. (Btw. that why I hate php, mixing markup and code is extremely obscure) The main problem seems to be the frequency of text-align-declarations in the existing style-sheets. This is what I have atm: <------------ snip -------------> diff --git a/web/html/css/containers.css b/web/html/css/containers.css index fc092de..fe7d26b 100644 --- a/web/html/css/containers.css +++ b/web/html/css/containers.css @@ -3,6 +3,12 @@ body,table,td,img { margin: 0; padding: 0; } + +/* Language specific formatting */ +body:lang(hu) { + direction:rtl; +} + /* Main Wrapper Data Format */ td.preHeader { background-color: #000; <--------------- snap ------------------> It would be nice to get some comments from right-to-left reading people. Why does direction:rtl; put punctuation at the beginning of the line? Is that wanted behavior or just a browser bug? -- irc: PyroPeter at freenode

PyroPeter

2:13 p.m.

On 08/14/2010 03:24 PM, PyroPeter wrote:

...

I just noticed you are setting xml:lang and lang on the <html>-tag. So there isn't even any change needed. (Btw. that why I hate php, mixing markup and code is extremely obscure)

The main problem seems to be the frequency of text-align-declarations in the existing style-sheets.

This is what I have atm:

(long diff is long)

...

It would be nice to get some comments from right-to-left reading people. Why does direction:rtl; put punctuation at the beginning of the line? Is that wanted behavior or just a browser bug?

I made a typo, correct patch: <-------- snip -------------> diff --git a/web/html/css/containers.css b/web/html/css/containers.css index fc092de..4707dac 100644 --- a/web/html/css/containers.css +++ b/web/html/css/containers.css @@ -3,6 +3,10 @@ body,table,td,img { margin: 0; padding: 0; } +/* Language specific formatting */ +body:lang(he) { + direction:rtl; +} /* Main Wrapper Data Format */ td.preHeader { background-color: #000; <-------- snap -------------> What distinguishes the four different css-files? What is the sense of having style-declarations in the html/php-soup? -- irc: PyroPeter at freenode

Loui Chang

2:28 p.m.

On Sat 14 Aug 2010 16:13 +0200, PyroPeter wrote:

...

On 08/14/2010 03:24 PM, PyroPeter wrote:

...
I just noticed you are setting xml:lang and lang on the <html>-tag. So there isn't even any change needed. (Btw. that why I hate php, mixing markup and code is extremely obscure)

The main problem seems to be the frequency of text-align-declarations in the existing style-sheets.

This is what I have atm:

(long diff is long)

...
It would be nice to get some comments from right-to-left reading people. Why does direction:rtl; put punctuation at the beginning of the line? Is that wanted behavior or just a browser bug?

I made a typo, correct patch: <-------- snip -------------> diff --git a/web/html/css/containers.css b/web/html/css/containers.css index fc092de..4707dac 100644 --- a/web/html/css/containers.css +++ b/web/html/css/containers.css @@ -3,6 +3,10 @@ body,table,td,img { margin: 0; padding: 0; } +/* Language specific formatting */ +body:lang(he) { + direction:rtl; +} /* Main Wrapper Data Format */ td.preHeader { background-color: #000; <-------- snap ------------->

What distinguishes the four different css-files? What is the sense of having style-declarations in the html/php-soup?

Beats me. It's just a result of people doing whatever needs doing without caring too much where things go. Believe me it was a lot worse. Things have been cleaned up a lot in the last couple years. Anyways if something in particular bothers you, try submitting a patch.

PyroPeter

15 Aug 15 Aug

12:29 p.m.

On 08/14/2010 04:28 PM, Loui Chang wrote:

...

Beats me. It's just a result of people doing whatever needs doing without caring too much where things go. Believe me it was a lot worse. Things have been cleaned up a lot in the last couple years.

Anyways if something in particular bothers you, try submitting a patch.

PyroPeter

11:53 p.m.

On 08/15/2010 02:29 PM, PyroPeter wrote:

...

This is what I have now:

(long patch)

...

As AUR is a system mainly used by users of a bleeding-edge distro, I assumed everyone is using a browser supporting CSS 3.

The search-results table looks pretty much like http://www.archlinux.org.il/packages/?q=foo now.

I will now start to check the package-details page.

Attached is a patch that adds Right-To-Left-support. From the commit message: This replaces most parts of pkg_search_results.php, as this file was the first one I looked at more closely that had great flaws: * The obscure page-number-generation-code was rewritten and moved to pkgfuncts.inc * There are no more "Next" and "Previous" buttons, imo they are redundant. * The links now cover the whole page range, not just the pages next to the current one. * The blind-table in the footer was replaced by floating divs to allow proper RTL-support. * The odd rows of the results-table are now made darker with means of CSS 3. Every arch-user's browser should support this. * The page is now valid HTML even if there are no results. * Removed all <td><span><span>content</span></span></td>-like oddities. <td> is an inline element by itself, there is no need to place a <span> in there. Two of them make even less sense. * Removed reoccurring style-attributes. Adopted CSS to do the same. * Changed indention to something (imo) senseful. Indention is now done like in HTML; If there is an 'if' or 'for' in the PHP-code, the affected HTML-elements are also indented. If someone considers my indention scheme to be senseful, I would add an explaination to HACKING. Regards, PyroPeter -- irc: PyroPeter at freenode

Loui Chang

16 Aug 16 Aug

3:47 a.m.

On Mon 16 Aug 2010 01:53 +0200, PyroPeter wrote:

...

Attached is a patch that adds Right-To-Left-support.

From the commit message: This replaces most parts of pkg_search_results.php, as this file was the first one I looked at more closely that had great flaws: * The obscure page-number-generation-code was rewritten and moved to pkgfuncts.inc * There are no more "Next" and "Previous" buttons, imo they are redundant. * The links now cover the whole page range, not just the pages next to the current one. * The blind-table in the footer was replaced by floating divs to allow proper RTL-support. * The odd rows of the results-table are now made darker with means of CSS 3. Every arch-user's browser should support this. * The page is now valid HTML even if there are no results. * Removed all <td><span><span>content</span></span></td>-like oddities. <td> is an inline element by itself, there is no need to place a <span> in there. Two of them make even less sense. * Removed reoccurring style-attributes. Adopted CSS to do the same. * Changed indention to something (imo) senseful. Indention is now done like in HTML; If there is an 'if' or 'for' in the PHP-code, the affected HTML-elements are also indented.

If someone considers my indention scheme to be senseful, I would add an explaination to HACKING.

I appreciate your efforts PyroPeter. I have some suggestions though. I recommend you to keep your patches small and focused. Here's a guideline: Each of those bullet points above should be at least one commit. This huge all-in-one patch makes it very hard for me to review, and frankly I don't like to deal with them at all. Try to send debatable patches last - for example I'm not so sure that removing 'next' and 'previous' is a good idea. So that applies when the patch is a matter of opinion. Thanks

PyroPeter

17 Aug 17 Aug

2:51 p.m.

On 08/16/2010 05:47 AM, Loui Chang wrote:

...

I appreciate your efforts PyroPeter. I have some suggestions though.

I recommend you to keep your patches small and focused.

Here's a guideline: Each of those bullet points above should be at least one commit.

This huge all-in-one patch makes it very hard for me to review, and frankly I don't like to deal with them at all.

Try to send debatable patches last - for example I'm not so sure that removing 'next' and 'previous' is a good idea. So that applies when the patch is a matter of opinion.

Thanks

Sorry for that, I will try to avoid that in the future. I am now splitting the commit into smaller ones. Attached are the first four patches: 0001. 00e497a pkg_search_results: rewrite of page navigation 0002. b65a5a3 pkg_search_results: increase size of links in page-navigation 0003. df02d42 pkg_search_results: replace blind-table with floating div's 0004. 76a874a Right-to-left written languages now supported ~~PyroPeter -- freenode/pyropeter "12:50 - Ich drücke Return."

PyroPeter

2 Sep 2 Sep

4:57 p.m.

On 08/17/2010 04:51 PM, PyroPeter wrote:

...

Sorry for that, I will try to avoid that in the future.

I am now splitting the commit into smaller ones. Attached are the first four patches: 0001. 00e497a pkg_search_results: rewrite of page navigation 0002. b65a5a3 pkg_search_results: increase size of links in page-navigation 0003. df02d42 pkg_search_results: replace blind-table with floating div's 0004. 76a874a Right-to-left written languages now supported

~~PyroPeter

Is there something wrong with the patches? Or do I just lack patience once again? -- freenode/pyropeter "12:50 - Ich drücke Return."

Loui Chang

3 Sep 3 Sep

3:10 a.m.

On Thu 02 Sep 2010 18:57 +0200, PyroPeter wrote:

...

On 08/17/2010 04:51 PM, PyroPeter wrote:

...
Sorry for that, I will try to avoid that in the future.

I am now splitting the commit into smaller ones. Attached are the first four patches: 0001. 00e497a pkg_search_results: rewrite of page navigation 0002. b65a5a3 pkg_search_results: increase size of links in page-navigation 0003. df02d42 pkg_search_results: replace blind-table with floating div's 0004. 76a874a Right-to-left written languages now supported

Is there something wrong with the patches? Or do I just lack patience once again?

Sorry, Thanks for resubmitting. I'm just lacking time to review them.

PyroPeter

30 Nov 30 Nov

6:55 p.m.

On 08/17/2010 04:51 PM, PyroPeter wrote:

...

On 08/16/2010 05:47 AM, Loui Chang wrote:

...
I appreciate your efforts PyroPeter. I have some suggestions though.

I recommend you to keep your patches small and focused.

Here's a guideline: Each of those bullet points above should be at least one commit.

This huge all-in-one patch makes it very hard for me to review, and frankly I don't like to deal with them at all.

Try to send debatable patches last - for example I'm not so sure that removing 'next' and 'previous' is a good idea. So that applies when the patch is a matter of opinion.

Thanks

Sorry for that, I will try to avoid that in the future.

I am now splitting the commit into smaller ones. Attached are the first four patches: 0001. 00e497a pkg_search_results: rewrite of page navigation 0002. b65a5a3 pkg_search_results: increase size of links in page-navigation 0003. df02d42 pkg_search_results: replace blind-table with floating div's 0004. 76a874a Right-to-left written languages now supported

~~PyroPeter

Maybe Lukas could review this patches, if Loui lacks the time to do it? -- freenode/pyropeter "12:50 - Ich drücke Return."

Lukas Fleischer

12 Feb 12 Feb

12:34 p.m.

Wow, I finally found some time to rebase and review all those patches... On Tue, Aug 17, 2010 at 04:51:03PM +0200, PyroPeter wrote:

...

I am now splitting the commit into smaller ones. Attached are the first four patches: 0001. 00e497a pkg_search_results: rewrite of page navigation

I have to admit that I don't really like that way of pagination. It just feels odd and counter-intuitive imho. Right now, we have "First", "Prev", "Next" and "Last" buttons as well as pages adjacent to the current one. If you want to find a package that you remember to start with the letter "r" and to contain the string "and" somewhere in the package description, typing that into the search criteria and browsing all results is not the right way to do it. The proper way to allow users to search for such packages is to provide more powerful search criteria, which is something we're already working on. Also, if we'd want to implement something like that, I'd prefer some kind of binary search. That might, in fact, be interesting :) Moving parts of the paging stuff to "pkgfuncts.inc" makes sense tho. I'll probably extract this from the patch.

...

0002. b65a5a3 pkg_search_results: increase size of links in page-navigation

I'm not sure about this. I've never heard any gripes about that links being too small. Is there any feature request concerning this?

...

0003. df02d42 pkg_search_results: replace blind-table with floating div's

Basically sounds like a good idea. I'll look into that!

...

0004. 76a874a Right-to-left written languages now supported

Basically looks ok, except that you revert some previous bug fixes. Also, where's that "css/languages.css" that you link to in the new header template? Thanks for your contributions!

PyroPeter

14 Feb 14 Feb

8:58 p.m.

On 02/12/2011 01:34 PM, Lukas Fleischer wrote:

...

Wow, I finally found some time to rebase and review all those patches...

...
0001. 00e497a pkg_search_results: rewrite of page navigation

I have to admit that I don't really like that way of pagination. It just feels odd and counter-intuitive imho. Right now, we have "First", "Prev", "Next" and "Last" buttons as well as pages adjacent to the current one. If you want to find a package that you remember to start with the letter "r" and to contain the string "and" somewhere in the package description, typing that into the search criteria and browsing all results is not the right way to do it. The proper way to allow users to search for such packages is to provide more powerful search criteria, which is something we're already working on. Also, if we'd want to implement something like that, I'd prefer some kind of binary search. That might, in fact, be interesting :)

Moving parts of the paging stuff to "pkgfuncts.inc" makes sense tho. I'll probably extract this from the patch.

I rewrote the pagination a second time, this time without changing the behavoir. Probably the world isn't ready yet for a commit that turns searching into something awesome (tm). (Children would have known what "logarithmic scaling" means even before doing their first polynomial division!) On the search criteria: it would be great to have regexps! Mysql even has support for that: SELECT * FROM foo WHERE foo REGEXP '^java';

...

...
0002. b65a5a3 pkg_search_results: increase size of links in page-navigation

I'm not sure about this. I've never heard any gripes about that links being too small. Is there any feature request concerning this?

No. But this only grews one-digit-links to be of the size of two-digit- links, so imo the usability gain outweights the space consumption. I merged this change into the patch mentioned above.

...

...
0003. df02d42 pkg_search_results: replace blind-table with floating div's

Basically sounds like a good idea. I'll look into that!

...
0004. 76a874a Right-to-left written languages now supported

Basically looks ok, except that you revert some previous bug fixes. Also, where's that "css/languages.css" that you link to in the new header template?

The last two patches probably need a lot of updating, I will do that next. I attached languages.css (forgot to git-add it)

...

Thanks for your contributions!

Thanks for reviewing them :-P -- freenode/pyropeter ETAOIN SHRDLU

Lukas Fleischer

10:01 p.m.

On Mon, Feb 14, 2011 at 09:58:50PM +0100, PyroPeter wrote:

...

I rewrote the pagination a second time, this time without changing the behavoir. Probably the world isn't ready yet for a commit that turns searching into something awesome (tm). (Children would have known what "logarithmic scaling" means even before doing their first polynomial division!)

Well, I don't get the point of it. The AUR is not a search engine and that logarithmic scaling feels a bit weird. But that's just an opinion.

...

On the search criteria: it would be great to have regexps! Mysql even has support for that: SELECT * FROM foo WHERE foo REGEXP '^java';

We decided not to implement this into the AUR backend. Quoting myself: "The AUR is a simple interface to upload, download and search for packages and shouldn't be regarded as almighty search engine. Also, there's no way to implement server-side regex search without any loss of security." The issue is about backtracking (or similar algorithms) taking a lot of time for specific expressions. RegEx search could be used to DoS the AUR easily. Furthermore, we believe that such functionality should rather be moved to the client side. We'll support that by providing easier access to package data (in the form of direct database access, database dumps, plain text lists of packages or something similar) in the future. Let's keep the AUR simple and clean, let the AUR helpers do the more complex stuff.

...

...
...
0002. b65a5a3 pkg_search_results: increase size of links in page-navigation

I'm not sure about this. I've never heard any gripes about that links being too small. Is there any feature request concerning this?

No. But this only grews one-digit-links to be of the size of two-digit- links, so imo the usability gain outweights the space consumption. I merged this change into the patch mentioned above.

I'll check that as soon as I get round to it.

...

...
...
0003. df02d42 pkg_search_results: replace blind-table with floating div's

Basically sounds like a good idea. I'll look into that!

...
0004. 76a874a Right-to-left written languages now supported

Basically looks ok, except that you revert some previous bug fixes. Also, where's that "css/languages.css" that you link to in the new header template?

The last two patches probably need a lot of updating, I will do that next. I attached languages.css (forgot to git-add it)

Cool. Thanks!

Lukas Fleischer

16 Feb 16 Feb

2:52 p.m.

On Mon, Feb 14, 2011 at 09:58:50PM +0100, PyroPeter wrote:

...

...
From 541c18f86d970051d837e9dd75e1122292d1fd4f Mon Sep 17 00:00:00 2001 From: PyroPeter Date: Mon, 14 Feb 2011 20:58:42 +0100 Subject: [PATCH] pkg_search_results: rewrite of pagination

* Most of the PHP-code was moved to pkgfuncs.php to keep the template simple.

Signed-off-by: PyroPeter --- web/html/css/arch.css | 17 +++++--- web/lib/pkgfuncs.inc | 28 +++++++++++++ web/template/pkg_search_results.php | 77 ++++++---------------------------- 3 files changed, 53 insertions(+), 69 deletions(-)

diff --git a/web/html/css/arch.css b/web/html/css/arch.css index c3ed3aa..203d9da 100644 --- a/web/html/css/arch.css +++ b/web/html/css/arch.css @@ -328,19 +328,24 @@ blockquote.code { text-decoration: none; }

-#pages { margin: 5px; } -#pages .page_num { +.pageNav { + margin:5px 0; +} +.pageNav .page_num { border: 1px solid #ddd; padding: 2px; color: #0771a6; }

Why don't you stick to the coding style and use lowerCamelCase instead of underscore_separated_class_names? Using non-standard capitalization doesn't contribute to the consistency of our code base.

...

- -#pages .page_num:hover { +.pageNav * { + display:inline-block; + text-align:center; + min-width:3ex; +} +.pageNav .page_num:hover { border: 1px solid #8faecd; color: #333; } - -#pages #page_sel { +.pageNav .page_sel { border: 1px solid #8faecd; padding: 2px; color: #333;

Same here. And again, you should have split this patch into two separate ones. There's no relation between CSS adjustments and the separation of action and view. Keep your patches small.

...

diff --git a/web/lib/pkgfuncs.inc b/web/lib/pkgfuncs.inc index 2f69321..0420054 100644 --- a/web/lib/pkgfuncs.inc +++ b/web/lib/pkgfuncs.inc @@ -559,6 +559,34 @@ function pkg_search_page($SID="") {

if ($total > 1 || $total == 0) { + # calculation of pagination links + $per_page = ($_GET['PP'] > 0) ? $_GET['PP'] : 25; + $current = ceil($first / $per_page); + $pages = ceil($total / $per_page); + $templ_pages = array(); + + if ($current - 1 > 1) + $templ_pages[__('First')] = 0; + + if ($current > 1) + $templ_pages[__('Previous')] = ($current - 2) * $per_page;

I'd just use the same criteria for both "First" and "Previous" buttons. It might be right that having separate "First" and "Previous" links on the second page doesn't make a lot of sense but those constantly appearing and disappering pagination links in search results are really confusing when actually browsing stuff imho. I'm open to different opinions tho.

...

+ + if ($current - 5 > 1) + $templ_pages[] = "ellipsis";

I'd prefer something like "$templ_pages['...'] = false;" here so we can just use "if ($pagestart === false) echo $pagenr;" to display text without ".page_sel" wrapper spans and don't need to deal with strange special cases later.

...

+ for ($i = max($current - 5, 1) ; $i <= min($pages, $current + 5) ; $i++) {

Please don't put spaces before those semicolons.

...

+ $templ_pages[$i] = ($i - 1) * $per_page; + } + + if ($current + 5 < $pages) + $templ_pages[] = "ellipsis";

Same as above.

...

+ + if ($current < $pages) + $templ_pages[__('Next')] = $current * $per_page; + + if ($current + 1 < $pages) + $templ_pages[__('Last')] = ($pages - 1) * $per_page;

Same as with "First" and "Previous".

...

+ include('pkg_search_form.php'); include('pkg_search_results.php'); } diff --git a/web/template/pkg_search_results.php b/web/template/pkg_search_results.php index 4830ca8..57d574c 100644 --- a/web/template/pkg_search_results.php +++ b/web/template/pkg_search_results.php @@ -113,73 +113,24 @@ for ($i = 0; $row = mysql_fetch_assoc($result); $i++) { </td>

<td align='right'> - <span class='f4'><span class='blue'> - <?php print __("Showing results %s - %s of %s", $first, $last, $total) ?> - </span></span> [...] - <?php echo __('Last') ?></a> - <?php endif; ?> - + <div class="pageStats f4 blue">

Where did you define ".pageStats"? You should use underscores as word separators here as well.

...

+ <?php print __("Showing results %s - %s of %s", $first, $last, $total) ?> + </div> + <div class="pageNav">

Use underscores, see above.

...

+ <?php foreach($templ_pages as $pagenr => $pagestart) { ?> + <?php if ($pagestart === "ellipsis") { ?> + ...

As mentioned above, I'd just use "if ($pagestart === false) echo $pagenr;" here and set page number offsets in "$templ_pages" to "false" if an entry doesn't link to a specific page range. That is a more generic solution.

...

+ <?php } else if ($pagestart + 1 == $first) { ?> + <span class="page_sel"><?php echo $pagenr ?></span> + <?php } else { ?> + <a class="page_num" href="packages.php?<?php print mkurl('O=' . ( $pagestart)) ?>"><?php echo $pagenr ?></a> + <?php } ?> + <?php } ?> </div> - </td> </tr>

-<?php - } -} -?> +<?php } ?> </table> </div> </form> -- 1.7.4.1

PyroPeter

6:51 p.m.

I changed the things you mentioned and also removed the CSS adjustments. Regards, PyroPeter

Lukas Fleischer

17 Feb 17 Feb

6:06 p.m.

On Wed, Feb 16, 2011 at 07:51:32PM +0100, PyroPeter wrote:

...

I changed the things you mentioned and also removed the CSS adjustments.

Thanks.

PyroPeter

15 Feb 15 Feb

7:16 p.m.

On 02/12/2011 01:34 PM, Lukas Fleischer wrote:

...

...
0003. df02d42 pkg_search_results: replace blind-table with floating div's

Basically sounds like a good idea. I'll look into that!

I finished updating this. I will now look into the next patch, actually adding rtl-support. Regards, PyroPeter -- freenode/pyropeter ETAOIN SHRDLU

Lukas Fleischer

16 Feb 16 Feb

3:07 p.m.

On Tue, Feb 15, 2011 at 08:16:16PM +0100, PyroPeter wrote:

...

...
From 65d170d638cc46f8552ba2aa6876efabeeec6397 Mon Sep 17 00:00:00 2001 From: PyroPeter Date: Tue, 15 Feb 2011 20:04:23 +0100 Subject: [PATCH 2/2] pkg_search_results: replace blind-table with floating divs

* I tried to remove errors in the sgml-structure e.g.: <div> <?php if (foo) { ?> </div> <?php } ?> * I did not remove or add code (except the <table> and <div> stuff, of cause). I only changed the order of the html/php-tags. * The bottom and top of the script are now properly indented. I did not indent the middle part (table of search results) because that would render the diff completely useless.

Signed-off-by: PyroPeter --- web/html/css/arch.css | 15 +++++ web/html/css/containers.css | 2 +- web/template/pkg_search_results.php | 116 +++++++++++++++++------------------ 3 files changed, 72 insertions(+), 61 deletions(-)

Could you please use underscore as delimiters instead of camelCase here as well?

PyroPeter

17 Feb 17 Feb

6:54 p.m.

On 02/16/2011 04:07 PM, Lukas Fleischer wrote:

...

Could you please use underscore as delimiters instead of camelCase here as well?

Done. -- freenode/pyropeter ETAOIN SHRDLU

Lukas Fleischer

18 Feb 18 Feb

2:45 p.m.

On Thu, Feb 17, 2011 at 07:54:36PM +0100, PyroPeter wrote:

...

On 02/16/2011 04:07 PM, Lukas Fleischer wrote:

...
Could you please use underscore as delimiters instead of camelCase here as well?

Done.

Thanks.

PyroPeter

15 Feb 15 Feb

8:45 p.m.

On 02/12/2011 01:34 PM, Lukas Fleischer wrote:

...

...
0004. 76a874a Right-to-left written languages now supported

Basically looks ok, except that you revert some previous bug fixes. Also, where's that "css/languages.css" that you link to in the new header template?

I now updated this, too. What bugfixes did I revert? I hope I did not revert them this time. I merged the content of the former languages.css into arch.css. Regards, PyroPeter -- freenode/pyropeter ETAOIN SHRDLU

PyroPeter

8:46 p.m.

On 02/15/2011 09:45 PM, PyroPeter wrote:

...

On 02/12/2011 01:34 PM, Lukas Fleischer wrote:

...
...
0004. 76a874a Right-to-left written languages now supported

Basically looks ok, except that you revert some previous bug fixes. Also, where's that "css/languages.css" that you link to in the new header template?

I now updated this, too. What bugfixes did I revert? I hope I did not revert them this time. I merged the content of the former languages.css into arch.css.

Regards, PyroPeter

Forgot to attach the patch. m( -- freenode/pyropeter ETAOIN SHRDLU

Lukas Fleischer

16 Feb 16 Feb

3:11 p.m.

On Tue, Feb 15, 2011 at 09:46:36PM +0100, PyroPeter wrote:

...

On 02/15/2011 09:45 PM, PyroPeter wrote:

...
On 02/12/2011 01:34 PM, Lukas Fleischer wrote:

...
...
0004. 76a874a Right-to-left written languages now supported

Basically looks ok, except that you revert some previous bug fixes. Also, where's that "css/languages.css" that you link to in the new header template?

I now updated this, too. What bugfixes did I revert? I hope I did not revert them this time. I merged the content of the former languages.css into arch.css.

Regards, PyroPeter

Forgot to attach the patch. m(

-- freenode/pyropeter ETAOIN SHRDLU

...

...
From 38e43b5e69ec2d8e15c0ef054ad09aa7f813940a Mon Sep 17 00:00:00 2001 From: PyroPeter Date: Tue, 15 Feb 2011 21:32:52 +0100 Subject: [PATCH 3/3] Support for langauges written right-to-left

Signed-off-by: PyroPeter --- web/html/css/arch.css | 51 +++++++++++++++++++++++++++++++++++++----- web/html/css/containers.css | 9 ------- web/template/footer.php | 2 +- 3 files changed, 46 insertions(+), 16 deletions(-)

Looks fine to me. I'll test that one soon. Thanks!

PyroPeter

17 Feb 17 Feb

6:58 p.m.

On 02/15/2011 09:45 PM, PyroPeter wrote:

...

I now updated this, too. What bugfixes did I revert? I hope I did not revert them this time. I merged the content of the former languages.css into arch.css.

Regards, PyroPeter

New version with s/camelCase/underscore_delimited/g -- freenode/pyropeter ETAOIN SHRDLU

Lukas Fleischer

18 Feb 18 Feb

2:45 p.m.

On Thu, Feb 17, 2011 at 07:58:13PM +0100, PyroPeter wrote:

...

On 02/15/2011 09:45 PM, PyroPeter wrote:

...
I now updated this, too. What bugfixes did I revert? I hope I did not revert them this time. I merged the content of the former languages.css into arch.css.

Regards, PyroPeter

New version with s/camelCase/underscore_delimited/g

Thanks.

4826

Age (days ago)

5021

Last active (days ago)

List overview

Download

40 comments

6 participants

participants (6)

foo bar
Loui Chang
Lukas Fleischer
Netanel Shine
Ng Oon-Ee
PyroPeter