# How to make a small tweak to free software

The target audience for this is people who are beginners at software engineering and using linux. A lot of the information here may be obvious or already known to you. The language involved is C but you do not need to know any C to read this tutorial. I used `mg` to write this blog post. I used vs code to edit the source code.

If you use a piece of free software and it's 99% perfect but there's just this one thing it does that annoys the hell out of you.. you can in theory just fix it! Here's a look at what doing that is like. Hopefully it inspires you, or you pick up a could tricks on the way!

## Step 0: Have a problem

This is the problem I'm having with the text editor I use called `mg`.

```
--**-Mg: no-newline-testfile.txt          (fundamental)--L1---------------------
No newline at end of file, add one? (y or n) 
```


It pesters me about adding a newline to the end of the file I'm working on. Either just add one or don't add one. I don't care! I just want to quit the program! Not as bad as vim though. 

## Step 1: Get the code, Build the code

The first thing to do is get the source code for the project and build it yourself, get a working version built from source. Do not assume that what you build from source will be the same as what you've installed with your linux package manager! 

From googling there's 3 obvious repositories for mg:

* http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.bin/mg/ "upstream" - this is the master copy. I don't know if this runs on linux. Not going to touch this. Especially since it's in CVS.
* https://github.com/hboetes/mg - This is what we're after.
* https://github.com/troglobit/mg - This seems like an interesting modernized version with lots of additions. Never seen this before but I may look into using it in future.

Now we pull down the latest source code. In many cases you will want to check out a specific release or development branch, sometimes the master branch is not the right thing at all. In this situation it's fine though. 

```
[river@river Hacking]$ git clone https://github.com/hboetes/mg.git
Cloning into 'mg'...
remote: Enumerating objects: 633, done.
remote: Counting objects: 100% (56/56), done.
remote: Compressing objects: 100% (15/15), done.
remote: Total 633 (delta 44), reused 45 (delta 41), pack-reused 577
Receiving objects: 100% (633/633), 339.19 KiB | 2.92 MiB/s, done.
Resolving deltas: 100% (386/386), done.
```

Find out what build system is used and how to build the software.

In this case we have a `Makefile` and just running `make` built the
thing! 

```
[river@river mg]$ make
cc -O2 -pipe -g -Wall -Wno-strict-aliasing -Wno-deprecated-declarations -DREGEX -D_GNU_SOURCE -isystem /usr/include/bsd -DLIBBSD_OVERLAY  -DHAVE_PTY_H -c autoexec.c
cc -O2 -pipe -g -Wall -Wno-strict-aliasing -Wno-deprecated-declarations -DREGEX -D_GNU_SOURCE -isystem /usr/include/bsd -DLIBBSD_OVERLAY  -DHAVE_PTY_H -c basic.c
cc -O2 -pipe -g -Wall -Wno-strict-aliasing -Wno-deprecated-declarations -DREGEX -D_GNU_SOURCE -isystem /usr/include/bsd -DLIBBSD_OVERLAY  -DHAVE_PTY_H -c bell.c
cc -O2 -pipe -g -Wall -Wno-strict-aliasing -Wno-deprecated-declarations -DREGEX -D_GNU_SOURCE -isystem /usr/include/bsd -DLIBBSD_OVERLAY  -DHAVE_PTY_H -c buffer.c
cc -O2 -pipe -g -Wall -Wno-strict-aliasing -Wno-deprecated-declarations -DREGEX -D_GNU_SOURCE -isystem /usr/include/bsd -DLIBBSD_OVERLAY  -DHAVE_PTY_H -c cinfo.c
cc -O2 -pipe -g -Wall -Wno-strict-aliasing -Wno-deprecated-declarations -DREGEX -D_GNU_SOURCE -isystem /usr/include/bsd -DLIBBSD_OVERLAY  -DHAVE_PTY_H -c dir.c
cc -O2 -pipe -g -Wall -Wno-strict-aliasing -Wno-deprecated-declarations -DREGEX -D_GNU_SOURCE -isystem /usr/include/bsd -DLIBBSD_OVERLAY  -DHAVE_PTY_H -c display.c
cc -O2 -pipe -g -Wall -Wno-strict-aliasing -Wno-deprecated-declarations -DREGEX -D_GNU_SOURCE -isystem /usr/include/bsd -DLIBBSD_OVERLAY  -DHAVE_PTY_H -c echo.c
cc -O2 -pipe -g -Wall -Wno-strict-aliasing -Wno-deprecated-declarations -DREGEX -D_GNU_SOURCE -isystem /usr/include/bsd -DLIBBSD_OVERLAY  -DHAVE_PTY_H -c extend.c
cc -O2 -pipe -g -Wall -Wno-strict-aliasing -Wno-deprecated-declarations -DREGEX -D_GNU_SOURCE -isystem /usr/include/bsd -DLIBBSD_OVERLAY  -DHAVE_PTY_H -c file.c
cc -O2 -pipe -g -Wall -Wno-strict-aliasing -Wno-deprecated-declarations -DREGEX -D_GNU_SOURCE -isystem /usr/include/bsd -DLIBBSD_OVERLAY  -DHAVE_PTY_H -c fileio.c
cc -O2 -pipe -g -Wall -Wno-strict-aliasing -Wno-deprecated-declarations -DREGEX -D_GNU_SOURCE -isystem /usr/include/bsd -DLIBBSD_OVERLAY  -DHAVE_PTY_H -c funmap.c
cc -O2 -pipe -g -Wall -Wno-strict-aliasing -Wno-deprecated-declarations -DREGEX -D_GNU_SOURCE -isystem /usr/include/bsd -DLIBBSD_OVERLAY  -DHAVE_PTY_H -c interpreter.c
interpreter.c: In function ‘expandvals’:
interpreter.c:625:18: warning: variable ‘excbuf’ set but not used [-Wunused-but-set-variable]
  625 |         char     excbuf[BUFSIZE], argbuf[BUFSIZE];
      |                  ^~~~~~
cc -O2 -pipe -g -Wall -Wno-strict-aliasing -Wno-deprecated-declarations -DREGEX -D_GNU_SOURCE -isystem /usr/include/bsd -DLIBBSD_OVERLAY  -DHAVE_PTY_H -c help.c
help.c: In function ‘showall’:
help.c:144:59: warning: ‘%s’ directive output may be truncated writing up to 79 bytes into a region of size 16 [-Wformat-truncation=]
  144 |                 (void)snprintf(keybuf, sizeof(keybuf), "%s%s ", prefix, buf);
      |                                                           ^~            ~~~
help.c:144:23: note: ‘snprintf’ output 2 or more bytes (assuming 81) into a destination of size 16
  144 |                 (void)snprintf(keybuf, sizeof(keybuf), "%s%s ", prefix, buf);
      |                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc -O2 -pipe -g -Wall -Wno-strict-aliasing -Wno-deprecated-declarations -DREGEX -D_GNU_SOURCE -isystem /usr/include/bsd -DLIBBSD_OVERLAY  -DHAVE_PTY_H -c kbd.c
cc -O2 -pipe -g -Wall -Wno-strict-aliasing -Wno-deprecated-declarations -DREGEX -D_GNU_SOURCE -isystem /usr/include/bsd -DLIBBSD_OVERLAY  -DHAVE_PTY_H -c keymap.c
cc -O2 -pipe -g -Wall -Wno-strict-aliasing -Wno-deprecated-declarations -DREGEX -D_GNU_SOURCE -isystem /usr/include/bsd -DLIBBSD_OVERLAY  -DHAVE_PTY_H -c line.c
cc -O2 -pipe -g -Wall -Wno-strict-aliasing -Wno-deprecated-declarations -DREGEX -D_GNU_SOURCE -isystem /usr/include/bsd -DLIBBSD_OVERLAY  -DHAVE_PTY_H -c macro.c
cc -O2 -pipe -g -Wall -Wno-strict-aliasing -Wno-deprecated-declarations -DREGEX -D_GNU_SOURCE -isystem /usr/include/bsd -DLIBBSD_OVERLAY  -DHAVE_PTY_H -c main.c
cc -O2 -pipe -g -Wall -Wno-strict-aliasing -Wno-deprecated-declarations -DREGEX -D_GNU_SOURCE -isystem /usr/include/bsd -DLIBBSD_OVERLAY  -DHAVE_PTY_H -c match.c
cc -O2 -pipe -g -Wall -Wno-strict-aliasing -Wno-deprecated-declarations -DREGEX -D_GNU_SOURCE -isystem /usr/include/bsd -DLIBBSD_OVERLAY  -DHAVE_PTY_H -c modes.c
cc -O2 -pipe -g -Wall -Wno-strict-aliasing -Wno-deprecated-declarations -DREGEX -D_GNU_SOURCE -isystem /usr/include/bsd -DLIBBSD_OVERLAY  -DHAVE_PTY_H -c paragraph.c
cc -O2 -pipe -g -Wall -Wno-strict-aliasing -Wno-deprecated-declarations -DREGEX -D_GNU_SOURCE -isystem /usr/include/bsd -DLIBBSD_OVERLAY  -DHAVE_PTY_H -c re_search.c
cc -O2 -pipe -g -Wall -Wno-strict-aliasing -Wno-deprecated-declarations -DREGEX -D_GNU_SOURCE -isystem /usr/include/bsd -DLIBBSD_OVERLAY  -DHAVE_PTY_H -c region.c
cc -O2 -pipe -g -Wall -Wno-strict-aliasing -Wno-deprecated-declarations -DREGEX -D_GNU_SOURCE -isystem /usr/include/bsd -DLIBBSD_OVERLAY  -DHAVE_PTY_H -c search.c
cc -O2 -pipe -g -Wall -Wno-strict-aliasing -Wno-deprecated-declarations -DREGEX -D_GNU_SOURCE -isystem /usr/include/bsd -DLIBBSD_OVERLAY  -DHAVE_PTY_H -c spawn.c
cc -O2 -pipe -g -Wall -Wno-strict-aliasing -Wno-deprecated-declarations -DREGEX -D_GNU_SOURCE -isystem /usr/include/bsd -DLIBBSD_OVERLAY  -DHAVE_PTY_H -c tty.c
cc -O2 -pipe -g -Wall -Wno-strict-aliasing -Wno-deprecated-declarations -DREGEX -D_GNU_SOURCE -isystem /usr/include/bsd -DLIBBSD_OVERLAY  -DHAVE_PTY_H -c ttyio.c
cc -O2 -pipe -g -Wall -Wno-strict-aliasing -Wno-deprecated-declarations -DREGEX -D_GNU_SOURCE -isystem /usr/include/bsd -DLIBBSD_OVERLAY  -DHAVE_PTY_H -c ttykbd.c
cc -O2 -pipe -g -Wall -Wno-strict-aliasing -Wno-deprecated-declarations -DREGEX -D_GNU_SOURCE -isystem /usr/include/bsd -DLIBBSD_OVERLAY  -DHAVE_PTY_H -c undo.c
cc -O2 -pipe -g -Wall -Wno-strict-aliasing -Wno-deprecated-declarations -DREGEX -D_GNU_SOURCE -isystem /usr/include/bsd -DLIBBSD_OVERLAY  -DHAVE_PTY_H -c util.c
cc -O2 -pipe -g -Wall -Wno-strict-aliasing -Wno-deprecated-declarations -DREGEX -D_GNU_SOURCE -isystem /usr/include/bsd -DLIBBSD_OVERLAY  -DHAVE_PTY_H -c version.c
cc -O2 -pipe -g -Wall -Wno-strict-aliasing -Wno-deprecated-declarations -DREGEX -D_GNU_SOURCE -isystem /usr/include/bsd -DLIBBSD_OVERLAY  -DHAVE_PTY_H -c window.c
cc -O2 -pipe -g -Wall -Wno-strict-aliasing -Wno-deprecated-declarations -DREGEX -D_GNU_SOURCE -isystem /usr/include/bsd -DLIBBSD_OVERLAY  -DHAVE_PTY_H -c word.c
cc -O2 -pipe -g -Wall -Wno-strict-aliasing -Wno-deprecated-declarations -DREGEX -D_GNU_SOURCE -isystem /usr/include/bsd -DLIBBSD_OVERLAY  -DHAVE_PTY_H -c yank.c
cc -O2 -pipe -g -Wall -Wno-strict-aliasing -Wno-deprecated-declarations -DREGEX -D_GNU_SOURCE -isystem /usr/include/bsd -DLIBBSD_OVERLAY  -DHAVE_PTY_H -c cmode.c
cc -O2 -pipe -g -Wall -Wno-strict-aliasing -Wno-deprecated-declarations -DREGEX -D_GNU_SOURCE -isystem /usr/include/bsd -DLIBBSD_OVERLAY  -DHAVE_PTY_H -c cscope.c
cc -O2 -pipe -g -Wall -Wno-strict-aliasing -Wno-deprecated-declarations -DREGEX -D_GNU_SOURCE -isystem /usr/include/bsd -DLIBBSD_OVERLAY  -DHAVE_PTY_H -c dired.c
cc -O2 -pipe -g -Wall -Wno-strict-aliasing -Wno-deprecated-declarations -DREGEX -D_GNU_SOURCE -isystem /usr/include/bsd -DLIBBSD_OVERLAY  -DHAVE_PTY_H -c grep.c
cc -O2 -pipe -g -Wall -Wno-strict-aliasing -Wno-deprecated-declarations -DREGEX -D_GNU_SOURCE -isystem /usr/include/bsd -DLIBBSD_OVERLAY  -DHAVE_PTY_H -c tags.c
cc  autoexec.o basic.o bell.o buffer.o cinfo.o dir.o display.o echo.o extend.o file.o fileio.o funmap.o interpreter.o help.o kbd.o keymap.o line.o macro.o main.o match.o modes.o paragraph.o re_search.o region.o search.o spawn.o tty.o ttyio.o ttykbd.o undo.o util.o version.o window.o word.o yank.o cmode.o cscope.o dired.o grep.o tags.o -o mg -lncursesw  -lbsd  -lutil
[river@river mg]$ ./mg -v
./mg: invalid option -- 'v'
```

This is good going! Normally this setup will be much more involved, take a lot of googling and looking into how to correctly invoke the build system tools. We got lucky today. 

## Step 2: Locate relevant portions of code.

```
[river@river mg]$ wc *.c
   116    363   2424 autoexec.c
   580   1965  12088 basic.c
    90    178   1201 bell.c
  1070   3768  23763 buffer.c
   158    914   4797 cinfo.c
   522   1801  10617 cmode.c
   611   1771  12632 cscope.c
   183    487   3496 dir.c
  1210   3712  26251 dired.c
  1081   4556  26841 display.c
  1020   3530  21527 echo.c
   942   3346  22160 extend.c
   778   2991  19141 file.c
   763   2638  16594 fileio.c
   336   1075   9996 funmap.c
   362   1081   7686 grep.c
   235    773   5228 help.c
   977   3389  22232 interpreter.c
   462   1471   9569 kbd.c
   577   1676   9211 keymap.c
   624   2315  14699 line.c
   408   1132   9031 log.c
   112    306   1970 macro.c
   347   1064   7351 main.c
   190    750   4376 match.c
   174    558   3555 modes.c
   499   1727  10953 paragraph.c
   687   2272  15082 region.c
   669   2283  15238 re_search.c
   855   2697  18504 search.c
    51    148   1003 spawn.c
   562   1751  11996 tags.c
   459   1719  10569 tty.c
   233    752   4726 ttyio.c
    80    234   1942 ttykbd.c
   606   1736  12196 undo.c
   529   1887  11405 util.c
    28     84    525 version.c
   440   1648  10139 window.c
   514   1568  10449 word.c
   267   1141   6636 yank.c
 20407  69257 449799 total
```

There's 20k lines of C code here. It is pretty much never possible to read and understand the entire source code of a project you are working on. That's not how software development works 

So what I'm going to do to find a relevant piece of code is simply search for the error message:  

```
[river@river mg]$ grep -R --include '*.c' 'No newline at end of file'
file.c:		eobnl = eyorn("No newline at end of file, add one");
```

Now in my code editor I can see this

```
	if (llength(lback(lpend)) != 0) {
		eobnl = eyorn("No newline at end of file, add one");
		if (eobnl != TRUE && eobnl != FALSE)
			return (eobnl); /* abort */
	}
```

So just looking at the function names alone. I can infer that the if condition is doing something alone the lines of counting how far away from the end the last newline is. If this is zero the file ends in a newline. I could just delete this block of code if I wanted to. Then it wont pester me. 

Lets test that out.

## Reproduce the defect

If I am going to make changes I need to test them. There needs to be a sequence of instructions that reproduces the bug/feature/problem. 

Here's what we can do:

```
[river@river mg]$ echo -n 'test' > /tmp/no-newline-testfile.txt && ./mg /tmp/no-newline-testfile.txt
```

press any letter to modify the file.

press control-x control-s to save it.

and we see:

```
'No newline at end of file, add one? (y or n)'
```

Now we can try deleting the block of code, build and test it out. and indeed the prompt is gone.

## Step 3: Come up with an appropriate solution

Deleting the block of code would be fine for my case alone. But a better solution would be to provide a setting that can go in the mg config file ~/.mg which can pick between the current behavior (which should be the default) and just saving without adding a newline or prompting about it. While we are there, it would make sense to also add an option to just always add the newline without prompting.

In the mg manual you can see an example ~/.mg config/startup file. It looks like this:

```
           global-set-key ")" self-insert-command
           global-set-key "\^x\^f" find-file
           global-set-key "\e[Z" backward-char
           set-default-mode fill
           set-fill-column 72
           auto-execute *.c c-mode
```

So we want to add our own command that's similar to set-default-mode,
let's call it set-newline-at-eof-mode and we will have 3 options:

* `prompt`
* `always`
* `never`

To add this feature to a completely new codebase that I have no experience with. The easiest thing to do is study how `set-default-mode` is implemented and copy it!

In funmap.c there is a table entry:
```
	{set_default_mode, "set-default-mode", 1},
```

The comment at the top of this table explains the meaning of the number 1:

```
/*
 * 3rd column in the functnames structure indicates how many
parameters the
 * function takes in 'normal' usage. This column is only used to identify
 * function profiles when lines of a buffer are being evaluated via excline().
 *
 *  0 = a toggle, non-modifiable insert/delete, region modifier, etc
 *  1 = value can be string or number value (like: file/buf name,
search string)
 *  2 = multiple type value required, see auto-execute, or
global-set-key, etc
 * -1 = error: interactive commmand, unsuitable for interpreter
 *
 * Some functions when used interactively may ask for a 'y' or 'n' (or another
 * character) to continue, in excline, a 'y' is assumed. Functions
like this
 * have '0' in the 3rd column below.
 */
```

so we will add our own table entry
```
	{set_newline_at_eof_mode, "set-newline-at-eof-mode", 1},
```

and we will need to implement our own function `set_newline_at_eof_mode` which just sets a variable to one of 3 values. We can do that following what the `set_default_mode` function does.

So here's the patch that I've come up with:

* https://github.com/rain-1/mg/commit/4570d41a5fb32a335b4400757f6ab77e64f0393a

Now we can build and test it manually and see if it works correctly. If so, set up our automatic tests.

## Step 4: Regession testing the new feature

Every time you fix a bug or implement a feature in software you should also create automatic regression tests that verify the fix or functionality. This means that any code changes or updates that occur in future will either not break/revert your work - or the fact that they do will be automatically caught and flagged up!

To test command line programs on linux you can use GNU expect. This doesn't work in this situation because we have an ncurses based application. What I've found works for testing something like that is to use tmux, it implements terminal emulator and has a capture feature which dumps the pane as a text file. So you can just grep the text file to check that it looks like what you expect.

* https://github.com/rain-1/mg/commit/10892a86b6c353237177599ec89755ff8f7d9580

So here I've got one script for each mode of the newline setting. It verifies that the prompt comes up when we want it, and doesn't come up in the 'never' and 'always' modes. It also verifies that the file was saved with the newline added (or not, depending).

```
[river@river test]$ ./go.sh
1..3
ok 1 - prompt
ok 2 - always
ok 3 - never
```

and with that we are done. Thanks for coming with on this journey today!