Noissue - [hgreview] Only use first line of commit message as review title

hgreview.py

 import BaseHTTPServer
 import os
 import re
 import socket
 import sys
 import urllib
 
 from mercurial import cmdutil, error
 
 SERVER = 'https://codereview.adblockplus.org'
@@ skipping 40 matching lines @@
     if opts.get('change'):
         rev = repo[opts['change']]
         args.extend(['--rev', '{}:{}'.format(rev.parents()[0], rev)])
     elif opts.get('revision'):
         args.extend(['--rev', opts['revision']])
     else:
         raise error.Abort('What should be reviewed? Either --revision or --change is required.')
 
     if not opts.get('issue'):
         # New issue, make sure title and message are set
-        if opts.get('change'):
-            description = repo[opts['change']].description()
-        else:
-            description = ui.prompt('New review title: ', '')

[Wladimir Palant, 2017/01/16 10:11:43]

The logic change here will have the effect that the prompt will appear even
though title is set explicitly on the command line. Please revert that unrelated
change, the original logic was correct.

[Vasily Kuznetsov, 2017/01/16 10:46:07]

Yes, that's right. I also missed this case when suggesting the change. After
playing around with different clever structures for this logic, I have this
proposal: how about we revert to the original code and add one line before line
74 (it was line 71 in original code: path = ....):

    opts['title'] = opts['title'].splitlines()[0]  # Title should be single line

Seems like it would do the trick.

[Wladimir Palant, 2017/01/16 21:50:50]

This isn't the right thing to do either. We should not cut off a title that was
passed in on the command line. Only the commit description we extract from
repository needs to be cut off - so the original commit did the right thing for
most part. Maybe something like this:

  fulltitle = None
  ...
  if not opts.get('title') and opts.get('change'):
    fulltitle = repo[opts['change']].description()
    opts['title'] = fulltitle.rstrip().split('\n')[0]
  ...
  if not opts.get('message'):
    opts['message'] = opts['title'] if fulltitle is None else fulltitle

[Vasily Kuznetsov, 2017/01/16 22:39:05]

My thinking was that it's quite unlikely that someone will pass a multiline
title on the command line or would even want to have a multiline title at all. I
also tried to not increase the complexity of this fragment (especially the
second proposal I think did a good job at that).

On the other hand you wrote this extension in the first place and you used it
more than me so I'm inclined to weigh your opinion more than mine. If you think
the use case with multiline title is important, I won't disagree.

[Wladimir Palant, 2017/01/17 10:30:17]

It's not impossible however - typically the data comes from a pipe then (e.g.
via xargs).

-        if not opts.get('title'):
-            # Drop everything but the first line of description in title
-            opts['title'] = description.rstrip().split('\n')[0]
+        fulltitle = None
+
+        if not opts.get('title') and not opts.get('change'):
+            opts['title'] = ui.prompt('New review title: ', '')
+        elif not opts.get('title'):
+            fulltitle = repo[opts['change']].description()
+            opts['title'] = fulltitle.rstrip().split('\n')[0]
+
         if not opts['title'].strip():
             raise error.Abort('No review title given.')
 
         if not opts.get('message'):
-            opts['message'] = description
+            opts['message'] = fulltitle or opts['title']
 
         path = (ui.config('paths', 'default-push')
                 or ui.config('paths', 'default')
                 or '')
         match = re.search(r'^(?:https://|ssh://hg@)(.*)', path)
         if match:
             opts['base_url'] = 'https://' + match.group(1)
 
         # Make sure there is at least one reviewer
         if not opts.get('reviewers'):
@@ skipping 76 matching lines @@
   </body>
 </html>
 ''' % port
 
     # Run the upload tool
     issue, patchset = scope['RealMain']([upload_path] + args)
 
     # Wait for the page to check in and retrieve issue URL
     if server:
         server.handle_request()