Saturday, July 28, 2018

kilo commentary: changing the interface is bad, especially when you don't need to

Step 122 begins a series of modifications that handles the Enter key inserting a new line. The first step is to rename editorAppendRow to editorInsertRow and add a new parameter.

Now this, in and of itself, is fine. Inserting a line at some arbitrary point in the file is a generalization of appending one at the end, so it makes sense to use the guts of editorAppendRow to create editorInsertRow.

Where things go off the rails (IMO) is in step 123, where we are supposed to go through the code, and modify all the old calls to editorAppendRow to the corresponding editorInsertRow. Now there are only two places to change, so this isn't a huge modification to the codebase. However, I see two problems with it:

  • It harms the readability of the code, editorAppendRow has a clearer meaning when we're specifically adding a line to the end of a file
  • It's actually unnecessary

We can create a new implementation of editorAppendRow quite simply:

static inline editorAppendRow(char* line, ssize_t linelen)
{ editorInsertRow(E.numrows, line, linelen); }

Much, much better

Monday, July 23, 2018

kilo commentary: In which an array of text lines turns out to be a bad design choice

Way back in step 55, the step was taken to store the file the editor is working on as an array of lines. Steps 119 through 125 are dealing with joining two lines (deleting at the start of a line) or creating a new line with the Enter key. Because of the array-of-lines data structure chosen many steps ago, these are both relatively complicated operations.

In an editor that treats a file simply as a sequence of characters, these operations are as easy (or hard) as inserting or deleting a character anywhere else in the file.

I am reminded of one of Fred Brooks' aphorisms, "plan to throw one away; you will, anyhow." kilo is not something you really fix so much as work through to identify all the mistakes to avoid the second time through.

Wednesday, July 18, 2018

kilo: The horrible, no good, very bad quit confirmation

Step 115 implements a quit confirmation. That is, the user is prevented from quitting kilo while there are unsaved changes, Well, at first:

    case CTRL_KEY('q'):
      if (E.dirty && quit_times > 0) {
        editorSetStatusMessage("WARNING!!! File has unsaved changes. "
          "Press Ctrl-Q %d more times to quit.", quit_times);
        quit_times--;
        return;
      }

Yeah, that's pretty awful UX right there. Ironically, we're just a few steps away from implementing an editorPrompt function (used to prompt for a filename to save to) which could also be used for saying "You have unsaved changes, really quit? (N/y)", which is how virtually every "real" editor does it.

Saturday, July 7, 2018

kilo commentary: Is this malloc Really Necessary?

Step 105 begins the process of saving the file to disk by mallocing a buffer the size of the entire file and then copying the file's text into it. Step 106 implements the code to write the contents of this buffer to a file.

There's really no good reason to do this. I suppose there's some speed gained by making a single call to write, rather than one for each line in the file. On the other hand, it seems unlikely to offset the cost of yet another dynamic allocation, and copying the entire contents of the file. A buffered writing system might be worth implementing, but this is not the way to do it.

kilo commentary: realloc is not magic

I'm now starting the "make this an actual text editor" part of implementing kilo. The first step, naturally, is to write code that inserts a new character. Here's the implementation of editorRowInsertChar given in step 101:

void editorRowInsertChar(erow *row, int at, int c) {
  if (at < 0 || at > row->size) at = row->size;
  row->chars = realloc(row->chars, row->size + 2);
  memmove(&row->chars[at + 1], &row->chars[at], row->size - at + 1);
  row->size++;
  row->chars[at] = c;
  editorUpdateRow(row);
}

So yeah, hence the title of this post. The call of memmove every time a character is inserted is pretty bad, too. But... well, this is actually probably good enough for a toy editor that no one is actually going to use for real editing work. Still, I wish the author had at least mentioned that this is very much a quick-and-dirty way to do it, and that there's better ways to do it.

Sunday, July 1, 2018

kilo milestone: finished chapter 4

Which is the longest chapter of the booklet, and also marks passing the halfway point (at least in number of steps).

One of the cliché interview questions is, "what weakness do you have?" (or some such). Well, one weakness for me is that I have uncounted projects that I have started, worked on for a bit, and then abandoned long before they were finished, or even really more than started. This kilo project was headed down that same path, when it languished untouched for a year.

I've determined to be more diligent about finishing my projects, or at least bringing them to a reasonable "unhooking point". And so I plan to continue on with the kilo project, at least through the final three chapters, if for no other reason than as an exercise in sticking with a project to the finish.