Help with contributing first change (patch for debbugs 354)

Help with contributing first change (patch for debbugs 354)

[Jeff Clough]

Hello,

TL;DR: I am attempting to contribute my first change to GNU Emacs and am
looking for a little hand-holding concerning the tools to use, and
someone to review my change.


I find myself with a surplus of free time and would like to spend some
of it contributing to GNU Emacs. As my knowledge of its guts is years
out of date (and was never that good to begin with), and I have only the
most *remedial* understanding of the tools involved, I decided to start
with a wishlist item and found debbugs:354
(http://debbugs.gnu.org/db/35/354.html).

This is a (seemingly well-received) proposal from 2008 which suggests
that append-to-file should output the number of characters appended in
addition to the name of the target file.

So, I did the following...

1. git checkout master
2. git pull
3. git checkout -b wishlist-354 (is this overkill for a small change?)
4. Changed write-region in "fileio.c"
5. Tested my change (is building in place advised?)
6. Created "wishlist354log.txt" with my commit message
7. git commit -a -F /path/to/wishlist354log.txt
8. git format-patch -1 (is using this advised?)

The last command created a file/email message which looks more or less
like what's between these lines of hyphens...

---------------------------
Changed write-region to show characters added in message

Fixes: debbugs:354
---
 src/fileio.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/src/fileio.c b/src/fileio.c
index 43ab456..42e9ffd 100644
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -5000,12 +5000,13 @@ write_region (Lisp_Object start, Lisp_Object end, Lisp_Object filename,
     }
 
   if (!auto_saving && !noninteractive)
-    message_with_string ((NUMBERP (append)
-			  ? "Updated %s"
-			  : ! NILP (append)
-			  ? "Added to %s"
-			  : "Wrote %s"),
-			 visit_file, 1);
+    message ((NUMBERP (append)
+	      ? "Updated %d characters of %s"
+	      : ! NILP (append)
+	      ? "Added %d characters to %s"
+	      : "Wrote %d characters to %s"),
+	     (int) (XINT (end) - XINT (start)),
+	     SDATA (ENCODE_SYSTEM (visit_file)));
 
   return Qnil;
 }
-- 
1.9.1
---------------------------

So, what's next?

Jeff

Re: Help with contributing first change (patch for debbugs 354)

[Paul Eggert]

Thanks for taking this on!  I'll follow up at Bug#354 and CC: you.

Re: Help with contributing first change (patch for debbugs 354)

[Eli Zaretskii]

> From: Jeff Clough <kb1vqh@gmail.com>
> Date: Thu, 26 Feb 2015 19:07:27 -0500
> 
> TL;DR: I am attempting to contribute my first change to GNU Emacs and am
> looking for a little hand-holding concerning the tools to use, and
> someone to review my change.

Thank you.

> 1. git checkout master
> 2. git pull
> 3. git checkout -b wishlist-354 (is this overkill for a small change?)

It's up to you.  I make such one-off changes directly in master, but
then I push them immediately after testing, while you will have to
wait until your patch is reviewed and approved.

> 4. Changed write-region in "fileio.c"
> 5. Tested my change (is building in place advised?)

Nothing wrong with building in place.  But please do make a point of
running the test suite, before and after the change, to make sure
there are no regressions.  Bonus points for adding tests to test this
new feature.

> So, what's next?

File a bug report with this patch, and wait for a review.

Thanks again.