GitLab integration take two, light edition

GitLab integration take two, light edition

[Adam Sjøgren]

A while back I mumbled about creating an nngitlab backend, and I got a
prototype working¹.

However, as predicted, it was quite slow, because checking for new
issues and comments meant an http-request per issue.

I kind of gave up on that (I should probably have tried if I could use
the GitLab API to ask for "changes since last update", but I ran out of
steam and was a little discouraged by a remark in the API documentation).

So today I lowered my level of ambition, and started by simply
implementing a function that gives a "thumbs up" to a comment, when
reading the notification email for the comment:

  (defun asjo-gitlab-give-thumbsup ()
    "Give thumbsup to current comment on a GitLab issue"
    (interactive)
    (with-current-buffer gnus-original-article-buffer
      (let ((project-id (gnus-fetch-field "X-GitLab-Project-Id"))
            (issue-iid (gnus-fetch-field "X-GitLab-Issue-IID"))
            (message-id (gnus-fetch-field "Message-ID")))
        (string-match "<note_\\([0-9]+\\)" message-id)
        (ghub-request "POST" (format "/projects/%s/issues/%s/notes/%s/award_emoji?name=thumbsup" project-id issue-iid (match-string 1 message-id)) nil :auth 'nngitlab :host "very.secret.gitlab.instance/api/v4" :username "asjo" :forge 'gitlab)
        (message "👍"))))

The new thing I wanted to do was to implement a "washing" function to
check what "award emojis" the comment I'm reading already has.

I found a comment in the manual saying that I can use
gnus-part-display-hook to add custom washing functions.

But I need some help. My feeble attempt was this:

  (defun asjo-article-treat-gitlab ()
    "Show thumbsup on GitLab comments"
    (let ((project-id (gnus-fetch-field "X-GitLab-Project-Id"))
          (issue-iid (gnus-fetch-field "X-GitLab-Issue-IID"))
          (message-id (gnus-fetch-field "Message-ID")))
      (when project-id
        (string-match "<note_\\([0-9]+\\)" message-id)
        (ignore-errors
          (let ((emoji (ghub-request "GET" (format "/projects/%s/issues/%s/notes/%s/award_emoji" project-id issue-iid (match-string 1 message-id)) nil :auth 'nngitlab :host "very.secret.gitlab.instance/api/v4" :username "asjo" :forge 'gitlab)))
            (message-goto-eoh)
            (insert "GitLab-Emoji: 👍\n")
            (message "👍"))))))

  (add-hook 'gnus-part-display-hook 'asjo-article-treat-gitlab)

There's a lot wrong here. It does a request for each part, so 3 or 4
(these are emails with both text and html MIME parts), and it puts in
the fake header in multiple places. I'm sure there are nice functions to
call to do what I want. Finding them is my issue.

Also, the request actually returns a list of the various emojis, which
ideally would be iterated over, and added to the header instead of the
hardcoded thumbs up.

Oh, and the ghub-request errors out with a 404 when there are no emojis
(funky API design), which really ought to insert an empty GitLab-Emoji:
fake header instead of just being ignored.

So the question is: how do I write a proper washing function for
"decorating" emails with the relevant X-GitLab-* headers which only does
one request per article?

Even with all the ailments of my code, it actually didn't feel slow, so
this "type" of "integration" might be a more viable way to spend less
time opening GitLab webpages and clicking various buttons.


  Best regards,

    Adam


¹ https://koldfront.dk/git/nngitlab/

-- 
 "En pessimist i sitt livs form"                            Adam Sjøgren
                                                       asjo@koldfront.dk

Re: GitLab integration take two, light edition

[Eric Abrahamsen]

Adam Sjøgren <asjo@koldfront.dk> writes:

> A while back I mumbled about creating an nngitlab backend, and I got a
> prototype working¹.

[...]

>   (defun asjo-article-treat-gitlab ()
>     "Show thumbsup on GitLab comments"
>     (let ((project-id (gnus-fetch-field "X-GitLab-Project-Id"))
>           (issue-iid (gnus-fetch-field "X-GitLab-Issue-IID"))
>           (message-id (gnus-fetch-field "Message-ID")))
>       (when project-id
>         (string-match "<note_\\([0-9]+\\)" message-id)
>         (ignore-errors
>           (let ((emoji (ghub-request "GET" (format "/projects/%s/issues/%s/notes/%s/award_emoji" project-id issue-iid (match-string 1 message-id)) nil :auth 'nngitlab :host "very.secret.gitlab.instance/api/v4" :username "asjo" :forge 'gitlab)))
>             (message-goto-eoh)
>             (insert "GitLab-Emoji: 👍\n")
>             (message "👍"))))))
>
>   (add-hook 'gnus-part-display-hook 'asjo-article-treat-gitlab)
>
> There's a lot wrong here. It does a request for each part, so 3 or 4
> (these are emails with both text and html MIME parts), and it puts in
> the fake header in multiple places. I'm sure there are nice functions to
> call to do what I want. Finding them is my issue.

I don't have any experience doing this so I'm also shooting in the dark
but...

It looks like the dynamic variable `gnus-treat-type' is bound while
`gnus-part-display-hook' is being run, so you should be able to examine
that to see if you're currently displaying "text/html" or what have you
(it could also be nil) and only run your function once.

Another approach would be to add your function to
`gnus-treatment-function-alist'.

> Also, the request actually returns a list of the various emojis, which
> ideally would be iterated over, and added to the header instead of the
> hardcoded thumbs up.
>
> Oh, and the ghub-request errors out with a 404 when there are no emojis
> (funky API design), which really ought to insert an empty GitLab-Emoji:
> fake header instead of just being ignored.

These two issues shouldn't be hard, right? What is the actual elisp
return value of `ghub-request'? If it's a list of codepoints, you can
just apply `string' to the list. If it's a list of strings, apply
`concat'. And wrap the whole thing in a `condition-case' to catch
whatever actual error represents a 404, and use a dummy string instead.

Is it more complicated than that for some reason?

(I'd also be curious to see where you gave up with the actual Gitlab
API -- do you have a relevant link you can share?)

Re: GitLab integration take two, light edition

[Adam Sjøgren]

Eric writes:

> I don't have any experience doing this so I'm also shooting in the dark
> but...

Appreciated!

> It looks like the dynamic variable `gnus-treat-type' is bound while
> `gnus-part-display-hook' is being run, so you should be able to examine
> that to see if you're currently displaying "text/html" or what have you
> (it could also be nil) and only run your function once.

Cool - I will check that out.

Ok, so now I can run it once, and I also figured out how to add a
header. Thanks for the push!

> Another approach would be to add your function to
> `gnus-treatment-function-alist'.

Yeah, making a "real" treatment would probably be the correctest (I've
heard that's a word) way to go.

>> Also, the request actually returns a list of the various emojis, which
>> ideally would be iterated over, and added to the header instead of the
>> hardcoded thumbs up.
>>
>> Oh, and the ghub-request errors out with a 404 when there are no emojis
>> (funky API design), which really ought to insert an empty GitLab-Emoji:
>> fake header instead of just being ignored.
>
> These two issues shouldn't be hard, right? What is the actual elisp
> return value of `ghub-request'?

It's just a list of lists of conses, so if only I could remember which
is which of car and cadr, it would indeed be easy.

This is what I have currently:

  (defun adsj-article-treat-gitlab ()
    "Show thumbsup on GitLab comments"
    (when (and gnus-treat-type (string= gnus-treat-type "text/html"))
      ;;; (gnus-with-article-buffer
      (let ((project-id (gnus-fetch-field "X-GitLab-Project-Id"))
            (issue-iid (gnus-fetch-field "X-GitLab-Issue-IID"))
            (message-id (gnus-fetch-field "Message-ID")))
        (when project-id
          (string-match "<note_\\([0-9]+\\)" message-id)
          (ignore-errors
            (let ((emojis (ghub-request "GET" (format "/projects/%s/issues/%s/notes/%s/award_emoji" project-id issue-iid (match-string 1 message-id)) nil :auth 'nngitlab :host "gitlab.nzcorp.net/api/v4" :username "adsj" :forge 'gitlab))
                  (name-to-emoji '(("thumbsup" . "👍"))))
              (widen)
              (message-goto-eoh)
              (dolist (emoji emojis)
                (let ((name (cdr (assoc-string "name" emoji))))
                  (insert "GitLab-Emoji: " (cdr (or (assoc-string name name-to-emoji) `("default" . ,name))) "\n"))))))))))

  (add-hook 'gnus-part-display-hook 'adsj-article-treat-gitlab)

The mapping of name-to-emoji needs to be expanded.

Ideally I'd rewrite it to do a map and join over the list, so I'd get a
single header with a comma-separated list of emojis, if there is more
than one.

The cherry on top would be if mousing over the emoji showed the name of
the person awarding the emoji (it's in the data returned). But it's
getting dark now.

I tried using a different hook, but then gnus-fetch-field returned nil.

And I cheated on handling the error, just ignoring all errors.

> Is it more complicated than that for some reason?

No, I just don't know (e)lisp, neither the vocabulary of functions nor
the idioms :-)

> (I'd also be curious to see where you gave up with the actual Gitlab
> API -- do you have a relevant link you can share?)

What made me give up was that I found a comment about the value of
last_activity_[something] not being updated. I can't find it again
currently.


  Best regards,

    Adam

-- 
 Interviewer: "So you don't fear Linux?"                    Adam Sjøgren
 Bill: "Oh - I - you know - my job is to fear          asjo@koldfront.dk
  everything."

Re: GitLab integration take two, light edition

[Eric Abrahamsen]

Adam Sjøgren <asjo@koldfront.dk> writes:

> Eric writes:
>

[...]

> It's just a list of lists of conses, so if only I could remember which
> is which of car and cadr, it would indeed be easy.
>
> This is what I have currently:
>
> (defun adsj-article-treat-gitlab ()
> "Show thumbsup on GitLab comments"
> (when (and gnus-treat-type (string= gnus-treat-type "text/html"))
>;;; (gnus-with-article-buffer
> (let ((project-id (gnus-fetch-field "X-GitLab-Project-Id"))
> (issue-iid (gnus-fetch-field "X-GitLab-Issue-IID"))
> (message-id (gnus-fetch-field "Message-ID")))
> (when project-id
> (string-match "<note_\\([0-9]+\\)" message-id)
> (ignore-errors
> (let ((emojis (ghub-request "GET" (format "/projects/%s/issues/%s/notes/%s/award_emoji" project-id issue-iid (match-string 1 message-id)) nil :auth 'nngitlab :host "gitlab.nzcorp.net/api/v4" :username "adsj" :forge 'gitlab))
> (name-to-emoji '(("thumbsup". "👍"))))
> (widen)
> (message-goto-eoh)
> (dolist (emoji emojis)
> (let ((name (cdr (assoc-string "name" emoji))))
> (insert "GitLab-Emoji: " (cdr (or (assoc-string name name-to-emoji) `("default" . ,name))) "\n"))))))))))
>
>   (add-hook 'gnus-part-display-hook 'adsj-article-treat-gitlab)
>
> The mapping of name-to-emoji needs to be expanded.
>
> Ideally I'd rewrite it to do a map and join over the list, so I'd get a
> single header with a comma-separated list of emojis, if there is more
> than one.

`mapconcat' is what you're looking for there, something like:

(insert "GitLab-Emoji: "
	(mapconcat
	 (lambda (emo)
	   (assoc-string emo name-to-emoji))
	 emojis))

With the appropriate guards and defaults, etc.

> The cherry on top would be if mousing over the emoji showed the name of
> the person awarding the emoji (it's in the data returned). But it's
> getting dark now.

Check out the "help-echo" text property in the manual.

> I tried using a different hook, but then gnus-fetch-field returned nil.
>
> And I cheated on handling the error, just ignoring all errors.
>
>> Is it more complicated than that for some reason?
>
> No, I just don't know (e)lisp, neither the vocabulary of functions nor
> the idioms :-)

Could have fooled me, you do pretty well with bug-hunting on this list!

Eric

Re: GitLab integration take two, light edition

[Eric Abrahamsen]

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> Adam Sjøgren <asjo@koldfront.dk> writes:
>
>> Eric writes:
>>
>
> [...]
>
>> It's just a list of lists of conses, so if only I could remember which
>> is which of car and cadr, it would indeed be easy.
>>
>> This is what I have currently:
>>
>> (defun adsj-article-treat-gitlab ()
>> "Show thumbsup on GitLab comments"
>> (when (and gnus-treat-type (string= gnus-treat-type "text/html"))
>>;;; (gnus-with-article-buffer
>> (let ((project-id (gnus-fetch-field "X-GitLab-Project-Id"))
>> (issue-iid (gnus-fetch-field "X-GitLab-Issue-IID"))
>> (message-id (gnus-fetch-field "Message-ID")))
>> (when project-id
>> (string-match "<note_\\([0-9]+\\)" message-id)
>> (ignore-errors
>> (let ((emojis (ghub-request "GET" (format "/projects/%s/issues/%s/notes/%s/award_emoji" project-id issue-iid (match-string 1 message-id)) nil :auth 'nngitlab :host "gitlab.nzcorp.net/api/v4" :username "adsj" :forge 'gitlab))
>> (name-to-emoji '(("thumbsup". "👍"))))
>> (widen)
>> (message-goto-eoh)
>> (dolist (emoji emojis)
>> (let ((name (cdr (assoc-string "name" emoji))))
>> (insert "GitLab-Emoji: " (cdr (or (assoc-string name name-to-emoji) `("default" . ,name))) "\n"))))))))))
>>
>>   (add-hook 'gnus-part-display-hook 'adsj-article-treat-gitlab)
>>
>> The mapping of name-to-emoji needs to be expanded.
>>
>> Ideally I'd rewrite it to do a map and join over the list, so I'd get a
>> single header with a comma-separated list of emojis, if there is more
>> than one.
>
> `mapconcat' is what you're looking for there, something like:
>
> (insert "GitLab-Emoji: "
> 	(mapconcat
> 	 (lambda (emo)
> 	   (assoc-string emo name-to-emoji))
> 	 emojis))

Bah, brain fart: mapconcat takes a third argument, the string to insert
between all the elements.

Re: GitLab integration take two, light edition

[Adam Sjøgren]

Eric writes:

> `mapconcat' is what you're looking for there, something like:

Yes, exactly what I needed - thanks!

This is what I have ended up with, so far, also showing Milestone,
Labels and State:

  (defun asjo-article-treat-gitlab ()
    "Show thumbsup on GitLab comments"
    (when (and gnus-treat-type (string= gnus-treat-type "text/html"))
      (let ((project-id (gnus-fetch-field "X-GitLab-Project-Id"))
            (issue-iid (gnus-fetch-field "X-GitLab-Issue-IID"))
            (message-id (gnus-fetch-field "Message-ID")))
        (when (and project-id issue-iid message-id)
          (let ((issue (ghub-request "GET" (format "/projects/%s/issues/%s" project-id issue-iid) nil :auth 'nngitlab :host "gitlab.example.org/api/v4" :username "asjo" :forge 'gitlab)))
            (let ((milestone (cdr (or (assoc "milestone" issue) '("default" . nil))))
                  (state (cdr (or (assoc-string "state" issue) '("default" . "unknown"))))
                  (labels (cdr (or (assoc-string "labels" issue) '("default" . nil)))))
              (widen)
              (message-goto-eoh)
              (when milestone
                (insert "GitLab-Milestone: " (cdr (or (assoc-string "title" milestone) '("default" . "unknown"))) "\n"))
              (when labels
                (insert "GitLab-Labels: " (mapconcat 'identity labels ", ") "\n"))
              (insert "GitLab-State: " state "\n")
              (string-match "<note_\\([0-9]+\\)" message-id)
              (ignore-errors
                (let ((emojis (ghub-request "GET" (format "/projects/%s/issues/%s/notes/%s/award_emoji" project-id issue-iid (match-string 1 message-id)) nil :auth 'nngitlab :host "gitlab.example.org/api/v4" :username "asjo" :forge 'gitlab))
                      (name-to-emoji '(("thumbsup" . "👍")
                                       ("thumbsdown" . "👎")
                                       ("smiley" . "😃"))))
                  (when emojis
                    (insert "GitLab-Emoji: " (mapconcat
                                              (lambda (x)
                                                (let ((name (cdr (assoc-string "name" x))))
                                                  (cdr (or (assoc-string name name-to-emoji) `("default" . ,name))))) emojis ", ") "\n"))))))))))

  (add-hook 'gnus-part-display-hook 'asjo-article-treat-gitlab)

and I'm pretty happy with that, it's even quite snappy, I don't notice
it running.

>> No, I just don't know (e)lisp, neither the vocabulary of functions nor
>> the idioms :-)
>
> Could have fooled me, you do pretty well with bug-hunting on this list!

That's just experience, which is much easier than knowledge.

> mapconcat takes a third argument, the string to insert between all the
> elements.

... and that was just what I needed!


  Thanks!

   Adam

-- 
 "the interview is very good, this man is smart and         Adam Sjøgren
  does not ask us idiot questions."                    asjo@koldfront.dk

Re: GitLab integration take two, light edition

[Eric Abrahamsen]

Adam Sjøgren <asjo@koldfront.dk> writes:

[...]


> (let ((milestone (cdr (or (assoc "milestone" issue) '("default" . nil))))
> (state (cdr (or (assoc-string "state" issue) '("default" . "unknown"))))
> (labels (cdr (or (assoc-string "labels" issue) '("default" . nil)))))
> (widen)
> (message-goto-eoh)
> (when milestone
> (insert "GitLab-Milestone: " (cdr (or (assoc-string
> "title" milestone) '("default". "unknown"))) "\n"))

This is all looking good, though there's some refactoring you could do
here (there's always refactoring that could be done) -- you've already
guarded against no milestone in the let, then you're checking "(when
milestone", which will always be true now, and then you do the guard all
over again in the body. I'm know you're still in the middle of whacking
this all together, but I thought it might be worth mentioning. In
general, the repetition of the

(cdr (or (assoc-string ..) '("default" . nil)))

pattern all over the place is a good indication that there's a cleaner
approach.

[...]

>>> No, I just don't know (e)lisp, neither the vocabulary of functions nor
>>> the idioms :-)
>>
>> Could have fooled me, you do pretty well with bug-hunting on this list!
>
> That's just experience, which is much easier than knowledge.

Hmm, that's debatable! But I take your point.

Re: GitLab integration take two, light edition

[Adam Sjøgren]

Eric writes:

> you've already guarded against no milestone in the let, then you're
> checking "(when milestone", which will always be true now, and then
> you do the guard all over again in the body. I'm know you're still in
> the middle of whacking this all together, but I thought it might be
> worth mentioning.

Indeed, thanks - yeah, I have been switching things around.

> In general, the repetition of the
>
> (cdr (or (assoc-string ..) '("default" . nil)))
>
> pattern all over the place is a good indication that there's a cleaner
> approach.

I was just happy to figure out what "cdr" does once more... :-D

Thanks for the comments!


  Best regards,

    Adam

-- 
 "*We* focus on doing nothing at all!"                      Adam Sjøgren
                                                       asjo@koldfront.dk