maubot / gitlab

A GitLab client and webhook receiver for maubot.
GNU Affero General Public License v3.0
95 stars 30 forks source link

Extraneous newlines before and after commits #5

Closed afranke closed 6 years ago

afranke commented 6 years ago

Commit notifications look like

{
  "origin_server_ts": 1523959677668,
  "sender": "@gitlab:t2bot.io",
  "event_id": "$152395967761790mbtyQ:t2bot.io",
  "unsigned": {
    "age": 13490826
  },
  "content": {
    "body": "\nInstall command with ninja, and fix new-release script (c47d98a4)\nRemove configure script (25a61ea9)\n\n",
    "msgtype": "m.notice",
    "formatted_body": "<ul>\n<li>Install command with ninja, and fix new-release script (c47d98a4)</li>\n<li>Remove configure script (25a61ea9)</li>\n</ul>\n",
    "format": "org.matrix.custom.html"
  },
  "type": "m.room.message",
  "room_id": "!hwiGbsdSTZIwSRfybq:matrix.org"
}

with unneeded \n before and after. Riot trims them but one shouldn’t rely on that behaviour. Please remove them.

tulir commented 6 years ago

Using Fprint instead of Fprintln in the push event handler should fix it. I'll see if I have some time to fix this and the other easy issues some time soon.

afranke commented 6 years ago

Awesome, thanks a lot!

afranke commented 6 years ago

The situation is a lot better, but there is still an extra \n at the end of the commit list because of the \n after the </li> in https://github.com/maubot/gitlab/blob/master/gitlab-webhook.go#L111

tulir commented 6 years ago

Removing that would very likely make the plaintext fallback much worse. Besides, ignoring newlines isn't something Riot does, newlines in HTML are ignored unless they're in a preformatted block

Anyway, I'm going to rewrite the bot to use the maubot platform soon, which uses Markdown for formatted messages and a more advanced plaintext fallback generator, so this will probably get fixed then.