notify.py: Use a/an correctly when sending request notifications

Message ID 20190809163702.12605-1-rustand.lars@gmail.com
State Superseded, archived
Headers show
Series
  • notify.py: Use a/an correctly when sending request notifications
Related show

Commit Message

Lars Rustand Aug. 9, 2019, 4:37 p.m. UTC
Will no longer send notifications about "a orphan request", but determine
whether to use a/an based on the first character of the request type.

Signed-off-by: Lars Rustand <rustand.lars@gmail.com>
---
 aurweb/scripts/notify.py | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Eli Schwartz Aug. 9, 2019, 5:33 p.m. UTC | #1
On 8/9/19 12:37 PM, Lars Rustand wrote:
> Will no longer send notifications about "a orphan request", but determine
> whether to use a/an based on the first character of the request type.

Thanks, looks like a reasonable change. See below for implementation
nitpicks.

> Signed-off-by: Lars Rustand <rustand.lars@gmail.com>
> ---
>  aurweb/scripts/notify.py | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/aurweb/scripts/notify.py b/aurweb/scripts/notify.py
> index d975086..ace6614 100755
> --- a/aurweb/scripts/notify.py
> +++ b/aurweb/scripts/notify.py
> @@ -414,8 +414,9 @@ class RequestOpenNotification(Notification):
>                     (self._user, self._pkgbase, self._merge_into)
>              body += '\n\n' + self._text
>          else:
> -            body = '%s [1] filed a %s request for %s [2]:' % \
> -                   (self._user, self._reqtype, self._pkgbase)
> +            an = ["a","an"][self._reqtype[0] in "aeiou"]

Using ['a', 'an'][True] as a list index by relying on it equaling the
`1` offset due to implicitly `int(True)` equaling `1` feels pretty
unreadable and I don't think I've ever seen that pattern, what about
instead using a standard ternary operator:

an = 'an' if self._reqtype[0] in 'aeiou' else 'a'

Also: the aurweb codebase generally uses single quotes (and does, for
the surrounding lines), so I think we should stick to that.

> +            body = '%s [1] filed %s %s request for %s [2]:' % \
> +                   (self._user, an, self._reqtype, self._pkgbase)
>              body += '\n\n' + self._text
>          return body
>  
>
Eli Schwartz Aug. 9, 2019, 6:59 p.m. UTC | #2
On 8/9/19 2:08 PM, Lars Rustand wrote:
> Argh, gmail replies to the wrong address.
> 
> On Fri, Aug 9, 2019, 20:06 Lars Rustand <rustand.lars@gmail.com> wrote:
> 
>> Thank you for the feedback, I totally agree with your remarks. I might be
>> a little too fond of codegolfing, that's where I got the idea to use True
>> as an index.
>> I posted an updated patch where I added your changes.

No problem. ;) Thanks.

As an aside, I have now seen int(True)-indexed lists used "in the wild"!
A clever fox saw what I wrote and PM'ed me a Fizzbuzz implementation
which relied on that.
Morten Linderud Aug. 9, 2019, 7:16 p.m. UTC | #3
On Fri, Aug 09, 2019 at 02:59:13PM -0400, Eli Schwartz wrote:
> On 8/9/19 2:08 PM, Lars Rustand wrote:
> > Argh, gmail replies to the wrong address.
> > 
> > On Fri, Aug 9, 2019, 20:06 Lars Rustand <rustand.lars@gmail.com> wrote:
> > 
> >> Thank you for the feedback, I totally agree with your remarks. I might be
> >> a little too fond of codegolfing, that's where I got the idea to use True
> >> as an index.
> >> I posted an updated patch where I added your changes.
> 
> No problem. ;) Thanks.
> 
> As an aside, I have now seen int(True)-indexed lists used "in the wild"!
> A clever fox saw what I wrote and PM'ed me a Fizzbuzz implementation
> which relied on that.

Pft. Implying it's not public on github!

https://gist.github.com/Foxboron/6643154

Patch

diff --git a/aurweb/scripts/notify.py b/aurweb/scripts/notify.py
index d975086..ace6614 100755
--- a/aurweb/scripts/notify.py
+++ b/aurweb/scripts/notify.py
@@ -414,8 +414,9 @@  class RequestOpenNotification(Notification):
                    (self._user, self._pkgbase, self._merge_into)
             body += '\n\n' + self._text
         else:
-            body = '%s [1] filed a %s request for %s [2]:' % \
-                   (self._user, self._reqtype, self._pkgbase)
+            an = ["a","an"][self._reqtype[0] in "aeiou"]
+            body = '%s [1] filed %s %s request for %s [2]:' % \
+                   (self._user, an, self._reqtype, self._pkgbase)
             body += '\n\n' + self._text
         return body