From patchwork Mon Mar 26 16:22:12 2018
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: D2944: wireproto: review fixups
From: phabricator <phabricator@mercurial-scm.org>
X-Patchwork-Id: 29867
Message-Id: <differential-rev-PHID-DREV-lkqtfosvtxfxb2vvuljs-req@phab.mercurial-scm.org>
To: mercurial-devel@mercurial-scm.org
Date: Mon, 26 Mar 2018 16:22:12 +0000

indygreg created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  Capture various TODOs and return an explicit value. This
  represents feedback from Yuya and Augie on various commits.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2944

AFFECTED FILES
  mercurial/help/internals/wireprotocol.txt
  mercurial/wireprotoserver.py

CHANGE DETAILS




To: indygreg, #hg-reviewers
Cc: mercurial-devel

diff --git a/mercurial/wireprotoserver.py b/mercurial/wireprotoserver.py
--- a/mercurial/wireprotoserver.py
+++ b/mercurial/wireprotoserver.py
@@ -353,6 +353,7 @@
         res.setbodybytes(_('invalid wire protocol command: %s') % command)
         return
 
+    # TODO consider cases where proxies may add additional Accept headers.
     if req.headers.get(b'Accept') != FRAMINGTYPE:
         res.status = b'406 Not Acceptable'
         res.headers[b'Content-Type'] = b'text/plain'
@@ -507,6 +508,7 @@
                              command['command'])
             return True
 
+        # TODO don't use assert here, since it may be elided by -O.
         assert authedperm in (b'ro', b'rw')
         wirecommand = wireproto.commands[command['command']]
         assert wirecommand.permission in ('push', 'pull')
@@ -556,7 +558,7 @@
         res.setbodygen(meta['framegen'])
         return True
     elif action == 'noop':
-        pass
+        return False
     else:
         raise error.ProgrammingError('unhandled event from reactor: %s' %
                                      action)
diff --git a/mercurial/help/internals/wireprotocol.txt b/mercurial/help/internals/wireprotocol.txt
--- a/mercurial/help/internals/wireprotocol.txt
+++ b/mercurial/help/internals/wireprotocol.txt
@@ -711,6 +711,8 @@
 * 0 or more UTF-8 strings that will be used as arguments to the
   formatting string.
 
+TODO use ASCII for formatting string.
+
 All data to be printed MUST be encoded into a single frame: this frame
 does not support spanning data across multiple frames.