integrations: Remove !avatar from git and game handler.

zulip/integrations/git/post-receive

@@ -17,7 +17,10 @@ import os.path
 
 sys.path.insert(0, os.path.dirname(__file__))
 import zulip_git_config as config
+
 VERSION = "0.9"
+EMPTY_SHA = "0000000000000000000000000000000000000000"
+COMMIT_ROW_TEMPLATE = '* {author_name} commited {subject} ([{commit_short_hash}]({commit_url}))\n'
 
 if config.ZULIP_API_PATH is not None:
     sys.path.append(config.ZULIP_API_PATH)
@@ -37,15 +40,26 @@ def git_repository_name() -> Text:
         return os.path.basename(os.path.dirname(os.getcwd()))
 
 def git_commit_range(oldrev: str, newrev: str) -> str:
+    remote_repo_cmd = ["git", "config", "--get", "remote.origin.url"]
+    remote_repo_url = subprocess.check_output(remote_repo_cmd).strip().decode("utf-8")
+
     log_cmd = ["git", "log", "--reverse",
-               "--pretty=%aE %H %s", "%s..%s" % (oldrev, newrev)]
+               "--pretty=%aN%n%H%n%h%n%s", "%s..%s" % (oldrev, newrev)]
     commits = ''
-    for ln in subprocess.check_output(log_cmd, universal_newlines=True).splitlines():
-        author_email, commit_id, subject  = ln.split(None, 2)
-        if hasattr(config, "format_commit_message"):
-            commits += config.format_commit_message(author_email, subject, commit_id)
-        else:
-            commits += '!avatar(%s) %s\n' % (author_email, subject)
+    output = subprocess.check_output(log_cmd, universal_newlines=True).splitlines()
+    it = iter(output)
+    for _ in range(len(output)//4):
+        author_name = next(iter(it))
+        commit_hash = next(iter(it))
+        commit_short_hash = next(iter(it))
+        subject = next(iter(it))
+        commits += COMMIT_ROW_TEMPLATE.format(
+            author_name=author_name,
+            commit_short_hash=commit_short_hash,
+            subject=subject,
+            commit_url="{}/commit/{}".format(remote_repo_url, commit_hash)
+        )
+
     return commits
 
 def send_bot_message(oldrev: str, newrev: str, refname: str) -> None:
@@ -59,20 +73,17 @@ def send_bot_message(oldrev: str, newrev: str, refname: str) -> None:
     new_head = newrev[:12]
     old_head = oldrev[:12]
 
-    if (
-        oldrev == '0000000000000000000000000000000000000000'
-        or newrev == '0000000000000000000000000000000000000000'
-    ):
+    if oldrev == EMPTY_SHA or newrev == EMPTY_SHA:
         # New branch pushed or old branch removed
         added = ''
         removed = ''
     else:
         added   = git_commit_range(oldrev, newrev)
         removed = git_commit_range(newrev, oldrev)
 
-    if oldrev == '0000000000000000000000000000000000000000':
+    if oldrev == EMPTY_SHA:
         message = '`%s` was pushed to new branch `%s`' % (new_head, branch)
-    elif newrev == '0000000000000000000000000000000000000000':
+    elif newrev == EMPTY_SHA:
         message = 'branch `%s` was removed (was `%s`)' % (branch, old_head)
     elif removed:
         message = '`%s` was pushed to `%s`, **REMOVING**:\n\n%s' % (new_head, branch, removed)

zulip/integrations/git/zulip_git_config.py

@@ -31,14 +31,6 @@ def commit_notice_destination(repo: Text, branch: Text, commit: Text) -> Optiona
     # Return None for cases where you don't want a notice sent
     return None
 
-# Modify this function to change how commits are displayed; the most
-# common customization is to include a link to the commit in your
-# graphical repository viewer, e.g.
-#
-# return '!avatar(%s) [%s](https://example.com/commits/%s)\n' % (author, subject, commit_id)
-def format_commit_message(author: Text, subject: Text, commit_id: Text) -> Text:
-    return '!avatar(%s) %s\n' % (author, subject)
-
 ## If properly installed, the Zulip API should be in your import
 ## path, but if not, set a custom path below
 ZULIP_API_PATH = None

zulip_bots/zulip_bots/bots/game_handler_bot/test_game_handler_bot.py

@@ -229,7 +229,7 @@ def test_quit_invite(self) -> None:
             }
         }
         self.verify_response(
-            'quit', 'Game cancelled.\n!avatar([email protected]) **foo** quit.', 0, bot, 'foo')
+            'quit', 'Game cancelled.\n**foo** quit.', 0, bot, 'foo')
 
     def test_user_already_in_game_errors(self) -> None:
         bot = self.setup_game()
@@ -313,14 +313,14 @@ def test_draw(self) -> None:
 
     def test_normal_turns(self) -> None:
         bot = self.setup_game()
-        self.verify_response('move 3', '**foo** moved in column 3\n\nfoo\n\n!avatar([email protected]) It\'s **baz**\'s (:red_circle:) turn.',
+        self.verify_response('move 3', '**foo** moved in column 3\n\nfoo\n\nIt\'s **baz**\'s (:red_circle:) turn.',
                              0, bot=bot, stream='test', subject='test game')
-        self.verify_response('move 3', '**baz** moved in column 3\n\nfoo\n\n!avatar([email protected]) It\'s **foo**\'s (:blue_circle:) turn.',
+        self.verify_response('move 3', '**baz** moved in column 3\n\nfoo\n\nIt\'s **foo**\'s (:blue_circle:) turn.',
                              0, bot=bot, stream='test', subject='test game', user_name='baz')
 
     def test_wrong_turn(self) -> None:
         bot = self.setup_game()
-        self.verify_response('move 5', '!avatar([email protected]) It\'s **foo**\'s (:blue_circle:) turn.', 0,
+        self.verify_response('move 5', 'It\'s **foo**\'s (:blue_circle:) turn.', 0,
                              bot=bot, stream='test', subject='test game', user_name='baz')
 
     def test_private_message_error(self) -> None:
@@ -390,7 +390,7 @@ def test_game_over_and_leaderboard(self) -> None:
         bot = self.setup_game()
         bot.put_user_cache()
         with patch('zulip_bots.bots.game_handler_bot.game_handler_bot.MockModel.determine_game_over', return_value='[email protected]'):
-            self.verify_response('move 3', '!avatar([email protected]) **foo** won! :tada:',
+            self.verify_response('move 3', '**foo** won! :tada:',
                                  1, bot=bot, stream='test', subject='test game')
         leaderboard = '**Most wins**\n\n\
 Player | Games Won | Games Drawn | Games Lost | Total Games\n\
@@ -403,12 +403,12 @@ def test_game_over_and_leaderboard(self) -> None:
     def test_current_turn_winner(self) -> None:
         bot = self.setup_game()
         with patch('zulip_bots.bots.game_handler_bot.game_handler_bot.MockModel.determine_game_over', return_value='current turn'):
-            self.verify_response('move 3', '!avatar([email protected]) **foo** won! :tada:',
+            self.verify_response('move 3', '**foo** won! :tada:',
                                  1, bot=bot, stream='test', subject='test game')
 
     def test_computer_turn(self) -> None:
         bot = self.setup_computer_game()
-        self.verify_response('move 3', '**foo** moved in column 3\n\nfoo\n\n!avatar([email protected]) It\'s **test-bot**\'s (:red_circle:) turn.',
+        self.verify_response('move 3', '**foo** moved in column 3\n\nfoo\n\nIt\'s **test-bot**\'s (:red_circle:) turn.',
                              0, bot=bot, stream='test', subject='test game')
         with patch('zulip_bots.bots.game_handler_bot.game_handler_bot.MockModel.determine_game_over', return_value='[email protected]'):
             self.verify_response('move 5', 'I won! Well Played!',
@@ -462,7 +462,6 @@ def test_get_game_info(self) -> None:
     def test_parse_message(self) -> None:
         bot = self.setup_game()
         self.verify_response('move 3', 'Join your game using the link below!\n\n> **Game `abc123`**\n\
-> !avatar([email protected])\n\
 > foo test game\n\
 > 2/2 players\n\
 > **[Join Game](/#narrow/stream/test/topic/test game)**', 0, bot=bot)
@@ -471,7 +470,6 @@ def test_parse_message(self) -> None:
 To move subjects, send your message again, otherwise join the game using the link below.
 
 > **Game `abc123`**
-> !avatar([email protected])
 > foo test game
 > 2/2 players
 > **[Join Game](/#narrow/stream/test/topic/test game)**''', 0, bot=bot, stream='test 2', subject='game 2')
@@ -485,7 +483,6 @@ def test_change_game_subject(self) -> None:
 To move subjects, send your message again, otherwise join the game using the link below.
 
 > **Game `abcdefg`**
-> !avatar([email protected])
 > foo test game
 > 2/2 players
 > **[Join Game](/#narrow/stream/test2/topic/test game 2)**''', 0, bot=bot, user_name='bar', stream='test game', subject='test2')

zulip_bots/zulip_bots/game_handler.py

@@ -139,7 +139,9 @@ def play_with_computer_help(self) -> str:
 
     def alert_new_invitation(self, game_id: str) -> str:
         # Since the first player invites, the challenger is always the first player
-        return '**' + self.get_host(game_id) + ' has invited you to play a game of ' + self.game_name + '.**\n' +\
+        player_email = self.get_players(game_id)[0]
+        sender_name = self.get_username_by_email(player_email)
+        return '**' + sender_name + ' has invited you to play a game of ' + self.game_name + '.**\n' +\
             self.get_formatted_game_object(game_id) + '\n\n' +\
             'Type ```accept``` to accept the game invitation\n' +\
             'Type ```decline``` to decline the game invitation.'
@@ -381,9 +383,8 @@ def command_quit(self, message: Dict[str, Any], sender: str, content: str) -> No
         if game_id == '':
             self.send_reply(
                 message, 'You are not in a game. Type `help` for all commands.')
-        sender_avatar = "!avatar({})".format(sender)
         sender_name = self.get_username_by_email(sender)
-        self.cancel_game(game_id, reason='{} **{}** quit.'.format(sender_avatar, sender_name))
+        self.cancel_game(game_id, reason='**{}** quit.'.format(sender_name))
 
     def command_join(self, message: Dict[str, Any], sender: str, content: str) -> None:
         if not self.is_user_not_player(sender, message):
@@ -485,8 +486,7 @@ def start_game(self, game_id: str) -> None:
     def get_formatted_game_object(self, game_id: str) -> str:
         object = '''> **Game `{}`**
 > {}
-> {}
-> {}/{} players'''.format(game_id, self.get_host(game_id), self.game_name, self.get_number_of_players(game_id), self.max_players)
+> {}/{} players'''.format(game_id, self.game_name, self.get_number_of_players(game_id), self.max_players)
         if game_id in self.instances.keys():
             instance = self.instances[game_id]
             if not self.is_single_player:
@@ -554,11 +554,6 @@ def get_number_of_players(self, game_id: str) -> int:
         num = len(self.get_players(game_id))
         return num
 
-    def get_host(self, game_id: str) -> str:
-        player_email = self.get_players(game_id)[0]
-        player_avatar = "!avatar({})".format(player_email)
-        return player_avatar
-
     def parse_message(self, message: Dict[str, Any]) -> None:
         game_id = self.is_user_in_game(message['sender_email'])
         game = self.get_game_info(game_id)
@@ -825,9 +820,7 @@ def handle_message(self, content: str, player_email: str) -> None:
             if self.gameAdapter.is_single_player:
                 self.broadcast('It\'s your turn')
             else:
-                user_turn_avatar = "!avatar({})".format(self.players[self.turn])
-                self.broadcast('{} It\'s **{}**\'s ({}) turn.'.format(
-                    user_turn_avatar,
+                self.broadcast('It\'s **{}**\'s ({}) turn.'.format(
                     self.gameAdapter.get_username_by_email(
                         self.players[self.turn]),
                     self.gameAdapter.gameMessageHandler.get_player_color(self.turn)))
@@ -889,9 +882,7 @@ def same_player_turn(self, content: str, message: str, is_computer: bool) -> Non
                 game_over = self.players[self.turn]
             self.end_game(game_over)
             return
-        user_turn_avatar = "!avatar({})".format(self.players[self.turn])
-        self.current_messages.append('{} It\'s **{}**\'s ({}) turn.'.format(
-            user_turn_avatar,
+        self.current_messages.append('It\'s **{}**\'s ({}) turn.'.format(
             self.gameAdapter.get_username_by_email(self.players[self.turn]),
             self.gameAdapter.gameMessageHandler.get_player_color(self.turn)
         ))
@@ -906,9 +897,7 @@ def next_turn(self) -> None:
         if self.gameAdapter.is_single_player:
             self.current_messages.append('It\'s your turn.')
         else:
-            user_turn_avatar = "!avatar({})".format(self.players[self.turn])
-            self.current_messages.append('{} It\'s **{}**\'s ({}) turn.'.format(
-                user_turn_avatar,
+            self.current_messages.append('It\'s **{}**\'s ({}) turn.'.format(
                 self.gameAdapter.get_username_by_email(self.players[self.turn]),
                 self.gameAdapter.gameMessageHandler.get_player_color(self.turn)
             ))
@@ -931,9 +920,8 @@ def end_game(self, winner: str) -> None:
         elif winner.startswith('except:'):
             loser = winner.lstrip('except:')
         else:
-            winner_avatar = "!avatar({})".format(winner)
             winner_name = self.gameAdapter.get_username_by_email(winner)
-            self.broadcast('{} **{}** won! :tada:'.format(winner_avatar, winner_name))
+            self.broadcast('**{}** won! :tada:'.format(winner_name))
         for u in self.players:
             values = {'total_games': 1, 'games_won': 0,
                       'games_lost': 0, 'games_drawn': 0}