kilo: I've started my own version of kilo

Based on the comments I've made so far, I've started my own version of kilo, which you can find here if you are interested.

I've chosen to make some different design and stylistic choices from the previous versions. Most notably, I just can't restrict myself to a single source file. Even for a relatively small project like this, it's just too claustrophobic. I've also chosen the Unix style of underscores in identifiers (editor_process_keypress) rather than camel case (editorProcessKeypress), which feels anachronistic to me in the C/Unix environment.

I've also created a simple Perl script to help deal with the string constant issue I've mentioned previously. This allows arcane terminal escape sequences to be named symbolically, with their length computed at compile time. I expect a decent C optimizer will generate very similar code to the original inline versions.

I've also renamed struct abuf to the typedef-ed term_buffer, so the erstwhile editorRefreshScreen looks like this in my implementation:

void editor_refresh_screen()
{
    term_buffer tb;
    tb_init(&tb);
    
    string_const home_cursor = get_home_cursor_str();

    tb_append_str(&tb, get_cursor_off_str());
    tb_append_str(&tb, home_cursor);

    editor_draw_rows(&tb);

    tb_append_str(&tb, home_cursor);
    tb_append_str(&tb, get_cursor_on_str());
    
    tb_write(&tb);
    tb_free(&tb);
}

The functions get_home_cursor_str(), get_cursor_off_str(), and so on are automatically generated by my Perl script linked above. Notice how much nicer this is than the version shown in step 40:

void editorRefreshScreen() {
  struct abuf ab = ABUF_INIT;

  abAppend(&ab, "\x1b[?25l", 6);
  abAppend(&ab, "\x1b[2J", 4);
  abAppend(&ab, "\x1b[H", 3);

  editorDrawRows(&ab);

  abAppend(&ab, "\x1b[H", 3);
  abAppend(&ab, "\x1b[?25h", 6);

  write(STDOUT_FILENO, ab.b, ab.len);
  abFree(&ab);
}

kilo commentary: getWindowSize (step 30)

In step 30 of implementing the kilo editor, this version of the getWindowSize function is presented:

int getWindowSize(int *rows, int *cols) {
  struct winsize ws;

  /* Note that the "1 ||" in the condition is deliberate, to force execution of the error arm of the if. */
  if (1 || ioctl(STDOUT_FILENO, TIOCGWINSZ, &ws) == -1 || ws.ws_col == 0) {
    if (write(STDOUT_FILENO, "\x1b[999C\x1b[999B", 12) != 12) return -1;
    editorReadKey();
    return -1;
  } else {
    *cols = ws.ws_col;
    *rows = ws.ws_row;
    return 0;
  }
}

This is a really ugly function, particularly considering how short it is. (For what it's worth, this is also where I got in reading through the kilo implementation when I decided I wanted to start blogging my reactions to it.)

First of all, there's another magic string and constant here, "\x1b[999C\x1b[999B" and this is definitely a place where it really feels like a bug waiting to happen. Another problem is that the outer if statement is structured backwards:

if (try something) {
  handle error case
} else {
  handle success case
}

I think it's best to keep the success case code close to the test, so having the error case code in between becomes problematic, especially as and when the error code becomes more complex. A second, related issue is that the error case code is essentially (the start of) an alternate strategy for retrieving the terminal dimensions, nested within the code for the first strategy. Notice how much nicer this reads with just a bit of refactoring:

int getWindowSize(int *rows, int* cols) {
  struct winsize ws;
  /* Note the De Morgan-ized condition to move the success condition to the "then" part of the if. */
  if (0 && ioctl(STDOUT_FILENO, TIOCGWINSZ, &ws) != -1 && ws.ws_col != 0) {
    *cols = ws.ws_col;
    *rows = ws.ws_row;
    return 0;
  }

  if (write(STDOUT_FILENO, "\x1b[999C\x1b[999B", 12) == 12) {
    editorReadKey();
  }

  return -1;
}

Now the two strategies for determining the terminal window size are separated, and the success code is right next to the test in both cases. Also note how much clearer it is here that the second strategy is incomplete, largely because it is no longer hidden inside the logic of the first strategy. Fix up the magic strings and numbers, and this code is vastly improved over the original presentation. Also note we appear to be doing a partial re-implementation of ncurses...

kilo commentary: editorDrawRows (step 29)

In step 25 and step 29 of implementing the kilo editor, a preliminary implementation of the function editorDrawRows is introducted:

void editorDrawRows() {
  int y;
  for (y = 0; y < E.screenrows; y++) {
    write(STDOUT_FILENO, "~\r\n", 3);
  }
}

This is meant to display a column of tildes on the left side of the screen, one on each row. But there's a bug, do you see what it is?

After writing out the final tide on the bottom most line of the terminal, it also outputs \r\n, carriage return line feed sequence, scrolling the screen. Of course, this is a major no-no for apps that want to control the screen as an editor like this does. It's not a huge deal in this very preliminary version of the code, and I assume it will be fixed later as we go, but it's something that bugged me when I noticed it.

The other (IMO more serious) problem with this (and other code in the kilo implementation) is the use of magic strings ("~\r\n") and numbers (3). This isn't that big of a deal in this tiny case, but as we'll see in my next post, the string constants aren't going to stay 3 characters long. A partial fix would be to introduce a simple wrapper function around write:

static inline ssize_t terminalWrite(const char* string)
{
  size_t sl = strlen(string);
  if ((size_t)write(STDOUT_FILENO, string, sl) == sl)
    return sl;
  return -1;
}

This will add the overhead of calling strlen every time a string is written, but particularly in an interactive environment, this should be a very minor issue. The magic string problem could be fixed by creating a list of global constants:

const char* g_tilde_rn = "~\r\n";
const char* g_tilde = "~";

We can then rewrite the function as follows:

void editorDrawRows() {
  int y;
  for (y = 0; y < E.screenrows; y++) {
    terminalWrite(y+1 < E.screenrows ? g_tilde_rn : g_tilde);
  }
}

Update: the extra \r\n bug is addressed in Step 35. This is a classic example of the fencepost version of the off-by-one type of bug.