Monday, June 4, 2018

kilo commentary:

Steps 80-84 of the kilo construction process have to do with rendering tab characters as spaces on the screen. Part of this is converting tab characters to the equivalent number of spaces, which is done by this bit of code:

      row->render[idx++] = ' ';
      while (idx % KILO_TAB_STOP != 0) row->render[idx++] = ' ';

Which... this is equivalent to this, right?

      do
          row->render[idx++] = ' ';
      while (idx % KILO_TAB_STOP != 0);

I mean, I realize that the do-while construct is relatively uncommon to use in C (at least in my experience), but this is a perfect example of where to use it.

Monday, May 28, 2018

kilo: revisiting the PAGE_UP and PAGE_DOWN keys

Back when I was implementing The PAGE_UP and PAGE_DOWN keys, I offered my own refactored implementation.

I've now just completed the steps to implement vertical scrolling in the text viewer phase of the project, I discover that my implementation no longer has the same effect once there is a file to be scrolled through. Fortunately, there is an easy fix:

    case PAGE_UP: 
        editor_move_cursor(0, -get_screen_height());
        break;
 
    case PAGE_DOWN:
        editor_move_cursor(0, get_screen_height());
        break;

As a side note, I found getting the scrolling code correct was surprisingly fiddly, due at least partially to my slightly different structure and very different naming conventions. I feel like there's some major structural refactoring that needs to be done, but I feel I'm already close to a place where many more changes will get me to the place where I can't map between the original version and my refactored version anymore. So I'm going to hold off on making any big changes at least until I work through the rest of the code.

Sunday, May 20, 2018

kilo: refactoring access to g_editor_state

I've been continuing working through the steps to implement kilo, but I've been increasingly unhappy with the accesses to the global g_editor_state variable all over the code; it's ugly and makes too much of the code dependent on the precise declaration of that struct.

So I decided to refactor so that all accesses to the editor state are through aptly named functions. I'm a lot happier with the result. As an example, here's the current implementation of editor_draw_rows:

void editor_draw_rows(term_buffer* tb)
{
    for (int y = 0; y < get_screen_height(); y++)
    {
        if (y < get_file_lines())
        {
            int len = get_line_size();
            if (len > get_screen_width())
              len = get_screen_width();
            tb_append(tb, get_line_chars(), len);
        }
        else if (y != get_screen_height()/3)
            tb_append_str(tb, get_tilde_str());
        else if (get_file_lines() == 0)
            append_welcome_message(tb);
        
        tb_append_str(tb, get_clear_row_str());
        if (y + 1 < get_screen_height())
            tb_append_str(tb, get_rn_str());
    }
}

Monday, May 14, 2018

kilo commentary: Page Up and Page Down buttons

Oh, dear. Way back when I started this commentary, I said I didn't want to be critical, but how the Page Up and Page Down buttons are implemented is pretty ugly:

    case PAGE_UP:
    case PAGE_DOWN:
      {
        int times = E.screenrows;
        while (times--)
          editorMoveCursor(c == PAGE_UP ? ARROW_UP : ARROW_DOWN);
      }
      break;

Particularly when the Home and End keys are implemented more like I would expect:

    case HOME_KEY:
      E.cx = 0;
      break;
    case END_KEY:
      E.cx = E.screencols - 1;
      break;

My code:

    case PAGE_UP: 
        editor_set_cursor_row(0);
        break;
 
    case PAGE_DOWN:
        editor_set_cursor_row(g_editor_state.screenrows-1);
        break;

The code for editorReadKey (the final version, at least for this chapter, is here) has also become pretty ugly. Even in this relatively short implementation, all these nested ifs and elses make it very hard to see how the control logic works.

That said, I don't want to take time right now to work out a cleaner solution than the one I have right now (I'm more interested in the other parts of this code) so I'm going to leave this the way I have it. Probably the best answer is to break down and use a third-party library that has already solved this problem.

Sunday, May 13, 2018

kilo commentary: Moving the cursor (steps 43-45)

In spite of a hiatus that's now more than a year long, I haven't forgotten about my kilo project. Steps 43-45 involve code to keep track of where the cursor is on the screen, and allow the user to move it around. Here's the original code that does that:

void editorMoveCursor(char key) {
  switch (key) {
    case 'a':
      E.cx--;
      break;
    case 'd':
      E.cx++;
      break;
    case 'w':
      E.cy--;
      break;
    case 's':
      E.cy++;
      break;
  }
}
void editorProcessKeypress() {
  char c = editorReadKey();
  switch (c) {
    case CTRL_KEY('q'):
      write(STDOUT_FILENO, "\x1b[2J", 4);
      write(STDOUT_FILENO, "\x1b[H", 3);
      exit(0);
      break;
    case 'w':
    case 's':
    case 'a':
    case 'd':
      editorMoveCursor(c);
      break;
  }
}

The code is basically decoding the w, s, a, and d characters twice: once to determine that the editorMoveCursor function should be called, and another to determine how the cursor should be moved. This is (IMO) another bug waiting to happen, in a large project these two points in the code could easily get out of sync. There's no reason not to do something like this (which is also my current solution):

void editor_move_cursor(int dcx, int dcy)
{
    g_editor_state.cx += dcx;
    g_editor_state.cy += dcy;
}


void editor_process_keypress(char c)
{
    string_const clear_screen = get_clear_screen_str();
    string_const home_cursor  = get_home_cursor_str();

    switch (c)
    {
    case CTRL_KEY('q'):
        write_str(clear_screen);
        write_str(home_cursor);
        exit(0);
        break;

    case 'w': editor_move_cursor( 0, -1); break;
    case 's': editor_move_cursor( 0,  1); break;
    case 'a': editor_move_cursor(-1,  0); break;
    case 'd': editor_move_cursor( 1,  0); break;
    }
}

Note that now only editor_process_keypress needs to know what keys move the cursor, and editor_move_cursor gets parameters that are directly interpretable.

Thursday, February 22, 2018

The interface to JavaScript's Date object is kinda broken

Here's an amusing thing to type into your favorite JavaScript console:

new Date('2018-01-01').getFullYear()

The response I get is 2017, which is surprising, at best.

What's happening?

The first part of the problem is that the Date object is actually a date and time object, and so new Date('2018-01-01') creates an object that has the date and time of January 1st, 2018 00:00 GMT. Which in and of itself is a reasonable enough choice.

The part that leads to the surprising result is that .getFullYear() returns a localized result, and since I am in a GMT+7 time zone, January 1st, 2018 00:00 GMT is actually December 31st, 2017 17:00 local time, and so .getFullYear() tells me it's still 2017.

Of course, the Date interface is broken in other ways too (.getYear() is actually non-Y2K compliant, in spite of the fact that it was presumably first designed and implemented less than a decade prior to the year 2000).

Can anything be done?

It turns out that dealing with time and time zones is actually surprisingly complex, and given JavaScript's "colorful" history, it's not surprising that the Date interface has its share of quirks. Really, I just wish there as an object for dealing with dates alone, without an accompanying time. Date would be a fine name for that object, if it weren't already taken.

I'm in a situation where I want to be able to deal with dates in JavaScript, and it seems like my choices are either to use the Date object with its rather non-intuitive interface, or roll my own date-only object. Neither choice seems optimal, but I think rolling my own date-only object would add more inertia to my projects than I'm willing to take on right now.