<div dir="ltr"><div class="gmail_quote">On Sun, Apr 4, 2010 at 1:03 AM, Mads Kiilerich <span dir="ltr">&lt;<a href="mailto:mads@kiilerich.com">mads@kiilerich.com</a>&gt;</span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">

Itamar Ravid wrote, On 04/03/2010 05:17 PM:<div class="im"><br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
# HG changeset patch<br>
# User Itamar Ravid&lt;<a href="mailto:iravid@iravid.com" target="_blank">iravid@iravid.com</a>&gt;<br>
# Date 1270307531 -10800<br>
# Node ID 2272a05026eb11f2d52067606729df6d86f18627<br>
# Parent  cd0c49bdbfd9edab18c3656d8a8a0bd27a9aa82a<br>
Abstract the functionality of calling an editor into the &#39;edit&#39; function to prevent<br>
users from having to mess with the code responsible for generating the temporary<br>
commit message and diff files.<br>
   <br>
</blockquote>
<br></div>
I don&#39;t know where it is documented, but the first line of the commit message is special by convention supported by Mercurial. IIRC it should be a summary of the change, no more than 80 characters, and preferable prefixed with (in this case) &quot;hgeditor:&quot;.<br>


<br>
[crew: Should the exact guideline (whatever it is) be added to ContributingChanges? (Perhaps together with some advice of content and language)]<div class="im"><br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
diff -r cd0c49bdbfd9 -r 2272a05026eb hgeditor<br>
--- a/hgeditor  Thu Apr 01 17:51:59 2010 -0500<br>
+++ b/hgeditor  Sat Apr 03 18:12:11 2010 +0300<br>
@@ -3,19 +3,22 @@<br>
  # This is an example of using HGEDITOR to create of diff to review the<br>
  # changes while commiting.<br>
<br>
-# If you want to pass your favourite editor some other parameters<br>
-# only for Mercurial, modify this:<br>
-case &quot;${EDITOR}&quot; in<br>
-    &quot;&quot;)<br>
-        EDITOR=&quot;vi&quot;<br>
-        ;;<br>
-    emacs)<br>
-        EDITOR=&quot;$EDITOR -nw&quot;<br>
-        ;;<br>
-    gvim|vim)<br>
-        EDITOR=&quot;$EDITOR -f -o&quot;<br>
-        ;;<br>
-esac<br>
+# Edit a file, optionally opening another one as reference within the editor.<br>
+# If you want to pass your favourite editor some other parameters only for Mercurial,<br>
+# modify the &#39;case&#39; statement within this function.<br>
+edit() {<br>
+    case &quot;${EDITOR}&quot; in<br>
+        &quot;&quot;)<br>
+            vi &quot;$1&quot; &quot;$2&quot;<br>
+            ;;<br>
+        emacs)<br>
+            emacs -nw &quot;$1&quot; &quot;$2&quot;<br>
+            ;;<br>
   <br>
</blockquote>
<br></div>
vi and emacs will now be called with an empty 2nd parameter in some cases (such as &quot;hg qref -e&quot;). vi will fail and emacs will complain. </blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">

<div class="im">The old behavior was that other values of $EDITOR could be used (such as vi) - that should be preserved.</div><div class="im"><br></div></blockquote><div><br></div><div>That&#39;s true - I&#39;ve overlooked those cases :-) Perhaps something like the suggestion below could be better?</div>

<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;"><div class="im">
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
  HGTMP=&quot;&quot;<br>
@@ -45,9 +48,9 @@<br>
      MD5=$(which md5 2&gt;/dev/null)<br>
  [ -x &quot;${MD5}&quot; ]&amp;&amp;  CHECKSUM=`${MD5} &quot;$HGTMP/msg&quot;`<br>
  if [ -s &quot;$HGTMP/diff&quot; ]; then<br>
-    $EDITOR &quot;$HGTMP/msg&quot; &quot;$HGTMP/diff&quot; || exit $?<br>
+    edit &quot;$HGTMP/msg&quot; &quot;$HGTMP/diff&quot; || exit $?<br>
  else<br>
-    $EDITOR &quot;$HGTMP/msg&quot; || exit $?<br>
+    edit &quot;$HGTMP/msg&quot; || exit $?<br>
  fi<br>
  [ -x &quot;${MD5}&quot; ]&amp;&amp;  (echo &quot;$CHECKSUM&quot; | ${MD5} -c&gt;/dev/null 2&gt;&amp;1&amp;&amp;  exit 13)<br>
   <br>
</blockquote>
<br></div>
It seems like this refactoring isn&#39;t a good idea.<br>
<br>
Perhaps there is no simple way to achieve what you want, so perhaps it isn&#39;t a good idea to include it in Mercurial?<br>
<br>
Or perhaps do it in a different hackish way: Define a &quot;vim&quot; function which handles the second argument in this special way and calls out to the binary.<br><font color="#888888">
<br>
/Mads<br></font></blockquote><div><br></div><div>edit() {</div><div>    case &quot;${EDITOR}&quot; in</div><div>        &quot;&quot;)</div><div>            EDITOR=&quot;vi&quot;</div><div>            ;;</div><div>        emacs)</div>

<div>            EDITOR=&quot;$EDITOR -nw&quot;</div><div>            ;;</div><div>        gvim|vim)</div><div>            # Note special case when opening two files below</div><div>            EDITOR=&quot;$EDITOR&quot;</div>

<div>            ;;</div><div>    esac</div><div><br></div><div>    if [ -z &quot;$2&quot; ]; then</div><div>        &quot;$EDITOR&quot; &quot;$1&quot;</div><div>    else</div><div>        if [ $EDITOR = &quot;vim&quot; -o $EDITOR = &quot;gvim&quot; ]; then</div>

<div>            # Handle irregular arg-ordering when calling g/vim</div><div>            &quot;$EDITOR&quot; &quot;+e $2&quot; &quot;+set buftype=help&quot; &quot;+split $1&quot;</div><div>        else</div><div>            &quot;$EDITOR&quot; &quot;$1&quot; &quot;$2&quot;</div>

<div>        fi</div><div>    fi</div><div>}</div><div> </div></div>This, I believe, handles the problems discussed while still containing the edit logic within the function. What do you think?<div><br></div><div>Regarding the commit message convention, I&#39;ll resubmit the patchbomb with a proper one once we iron out the code issues.</div>

</div>