5
\$\begingroup\$

Working on a project, I've taken to adapting the linenoise line editing library for my own use, among other things, rewriting it using C++. The idea is to separate my changes and updates into a new library, and publish it on GitHub, as one does. But before I release my changes into the wild, I'd like to ensure that it is, you know, good.

What I would like is general notes on correctness and practices.

(History not being implemented is a known issue.)

linenoise.h

/* linenoise.h -- guerrilla line editing library against the idea that a
 * line editing lib needs to be 20,000 lines of C code.
 *
 * See linenoise.cpp for more information.
 *
 * ------------------------------------------------------------------------
 *
 * Copyright (c) 2014-2014, Williham Totland <williham dot totland at gmail dot com>
 * Copyright (c) 2010-2014, Salvatore Sanfilippo <antirez at gmail dot com>
 * Copyright (c) 2010-2013, Pieter Noordhuis <pcnoordhuis at gmail dot com>
 *
 * All rights reserved.
 *
 * BSD 2-term, abbreviated for clarity.
 */

#import <unistd.h>

#import <unordered_set>
#import <string>
#import <deque>

namespace linenoise {

  typedef struct termios termiosSettings;

  class terminal {
    friend class editor;

    std::unordered_set<std::string> unsupportedTerminals;
    int inputFileDescriptor, outputFileDescriptor;

    termiosSettings *original, *rawMode;
    bool initializeRawMode ();

    size_t cursorPosition ();
    size_t columns ();

    void clearScreen ();
  public:
    terminal(const std::unordered_set<std::string> &uT, int ifd, int ofd) : unsupportedTerminals(uT), inputFileDescriptor(ifd), outputFileDescriptor(ofd) {
      initializeRawMode();
    }

    bool supported () const;

    bool enableRawMode (int);
    void disableRawMode (int);

    static void beep ();
  };

  class editor {
    enum class keyAction;

    class cursor {
      friend class editor;

      const editor &editor;

      size_t position;
    public:
      cursor(const class editor &e) : editor(e), position(0) { }

      void home (), left (), right (), end ();
    };

    cursor cursor;

    class history : public std::deque<const std::string> {
      friend class editor;

      void add (const std::string &);

      void save (const std::string &), load (const std::string &);
    };

    history history;
    ssize_t hCursor;

    std::string lineBuffer;
    std::string killBuffer = "";

    terminal term;
    bool hasSupportedTerminal;

    bool edit (int, int);

    void insert (char c);

    void kill (bool line), yank (), swap ();
    void deleteBack (), deleteForward (), deleteWord ();

    void historyPrev (), historyNext ();

    size_t completeLine ();
  public:
    std::string prompt;
    bool done = false;

    editor() : cursor(*this), term({"dumb", "cons25", "emacs", ""}, STDIN_FILENO, STDOUT_FILENO), hasSupportedTerminal(term.supported()) { }

    std::string getline ();

    void refresh ();
  };

}

linenoise.cpp

/* linenoise.cpp -- guerrilla line editing library against the idea that a
 * line editing lib needs to be 20,000 lines of C code.
 *
 * You can find the latest source code at:
 *
 *   http://github.com/antirez/linenoise
 *
 * Does a number of crazy assumptions that happen to be true in 99.9999% of
 * the 2010 UNIX computers around.
 *
 * ------------------------------------------------------------------------
 *
 * Copyright (c) 2014-2014, Williham Totland <williham dot totland at gmail dot com>
 * Copyright (c) 2010-2014, Salvatore Sanfilippo <antirez at gmail dot com>
 * Copyright (c) 2010-2013, Pieter Noordhuis <pcnoordhuis at gmail dot com>
 *
 * All rights reserved.
 *
 * BSD 2-term, abbreviated for clarity
 *
 * ------------------------------------------------------------------------
 *
 * References:
 * - http://invisible-island.net/xterm/ctlseqs/ctlseqs.html
 * - http://www.3waylabs.com/nw/WWW/products/wizcon/vt220.html
 *
 * Todo list:
 * - Filter bogus Ctrl+<char> combinations.
 * - Win32 support
 *
 * Bloat:
 * - History search like Ctrl+r in readline?
 *
 * List of escape sequences used by this program, we do everything just
 * with three sequences. In order to be so cheap we may have some
 * flickering effect with some slow terminal, but the lesser sequences
 * the more compatible.
 *
 * EL (Erase Line)
 *    Sequence: ESC [ n K
 *    Effect: if n is 0 or missing, clear from cursor to end of line
 *    Effect: if n is 1, clear from beginning of line to cursor
 *    Effect: if n is 2, clear entire line
 *
 * CUF (CUrsor Forward)
 *    Sequence: ESC [ n C
 *    Effect: moves cursor forward n chars
 *
 * CUB (CUrsor Backward)
 *    Sequence: ESC [ n D
 *    Effect: moves cursor backward n chars
 *
 * The following is used to get the terminal width if getting
 * the width with the TIOCGWINSZ ioctl fails
 *
 * DSR (Device Status Report)
 *    Sequence: ESC [ 6 n
 *    Effect: reports the current cusor position as ESC [ n ; m R
 *            where n is the row and m is the column
 *
 * When multi line mode is enabled, we also use an additional escape
 * sequence. However multi line editing is disabled by default.
 *
 * CUU (Cursor Up)
 *    Sequence: ESC [ n A
 *    Effect: moves cursor up of n chars.
 *
 * CUD (Cursor Down)
 *    Sequence: ESC [ n B
 *    Effect: moves cursor down of n chars.
 *
 * When linenoiseClearScreen() is called, two additional escape sequences
 * are used in order to clear the screen and position the cursor at home
 * position.
 *
 * CUP (Cursor position)
 *    Sequence: ESC [ H
 *    Effect: moves the cursor to upper left corner
 *
 * ED (Erase display)
 *    Sequence: ESC [ 2 J
 *    Effect: clear the whole screen
 *
 */

#include <termios.h>
#include <unistd.h>
#include <stdlib.h>
#include <stdio.h>
#include <errno.h>
#include <string.h>
#include <stdlib.h>
#include <ctype.h>
#include <sys/types.h>
#include <sys/ioctl.h>
#include <unistd.h>

#import <string>
#import <sstream>
#import <unordered_set>
#import <iostream>

#import "linenoise.h"

namespace linenoise {

  enum class editor::keyAction {
    KEY_NULL  =   0,
    CTRL_A    =   1,
    CTRL_B    =   2,
    CTRL_C    =   3,
    CTRL_D    =   4,
    CTRL_E    =   5,
    CTRL_F    =   6,
    CTRL_H    =   8,
    CTRL_I    =   9,
    TAB       =   9,
    CTRL_K    =  11,
    CTRL_L    =  12,
    ENTER     =  13,
    CTRL_N    =  14,
    CTRL_P    =  16,
    CTRL_T    =  20,
    CTRL_U    =  21,
    CTRL_W    =  23,
    CTRL_Y    =  25,
    ESC       =  27,
    BACKSPACE = 127,
  };

#pragma mark linenoise::terminal

  bool terminal::supported () const {
    const char *cterm(getenv("TERM"));
    const std::string term(cterm?cterm:"");

    return (unsupportedTerminals.find(term) == unsupportedTerminals.end());
  }

  bool terminal::initializeRawMode () {
    if (!isatty(inputFileDescriptor))
      return false;

    original = (termiosSettings *)malloc(sizeof(termiosSettings));
    tcgetattr(inputFileDescriptor, original);
    rawMode  = (termiosSettings *)malloc(sizeof(termiosSettings));
    rawMode  = (termiosSettings *)memcpy(rawMode, original, sizeof(termiosSettings));

    termiosSettings &raw = *rawMode;
    raw.c_iflag &= ~(BRKINT | ICRNL | INPCK | ISTRIP | IXON);
    raw.c_oflag &= ~(OPOST);
    raw.c_cflag |= (CS8);
    raw.c_lflag &= ~(ECHO | ICANON | IEXTEN | ISIG);
    raw.c_cc[VMIN] = 1; raw.c_cc[VTIME] = 0;

    return true;
  }

  bool terminal::enableRawMode (int fd) {
    if (tcsetattr(fd,TCSAFLUSH,rawMode) >= 0)
      return true;

    errno = ENOTTY;
    return false;
  }

  void terminal::disableRawMode (int fd) {
    tcsetattr(fd,TCSAFLUSH,original);
  }

  /* Use the ESC [6n escape sequence to query the horizontal cursor position
   * and return it. On error -1 is returned, on success the position of the
   * cursor. */
  size_t terminal::cursorPosition () {
    char buf[32];
    int cols, rows;
    unsigned int i = 0;

    if (write(outputFileDescriptor, "\x1b[6n", 4) != 4) return -1;

    /* Read the response: ESC [ rows ; cols R */
    while (i < sizeof(buf)-1) {
      if (read(inputFileDescriptor, buf + i, 1) != 1) break;
      if (buf[i] == 'R') break;
      i++;
    }
    buf[i] = '\0';

    if (sscanf(buf,"\x1b[%d;%d", &rows, &cols) != 2) return -1;
    return cols;
  }

  size_t terminal::columns () {
    struct winsize ws;

    if (ioctl(1, TIOCGWINSZ, &ws) == -1 || ws.ws_col == 0) {
      size_t start, cols;

      start = cursorPosition();

      write(outputFileDescriptor, "\x1b[999C", 6);
      cols = cursorPosition();

      if (cols > start) {
        char seq[32];
        snprintf(seq,32,"\x1b[%luD", cols - start);
        write(outputFileDescriptor, seq, strlen(seq));
      }

      return cols;
    } else {
      return ws.ws_col;
    }

    return 80;
  }

  void terminal::clearScreen () {
    write(outputFileDescriptor,"\x1b[H\x1b[2J",7);
  }

  void terminal::beep () {
    fprintf(stderr, "\x7");
    fflush(stderr);
  }

#pragma mark linenoise::editor::cursor

  void editor::cursor::home () {
    position = 0;
  }

  void editor::cursor::left () {
    size_t max = editor.lineBuffer.size();

    if (position > max)
      position = max;

    if (position > 0)
      --position;
  }

  void editor::cursor::right () {
    size_t max = editor.lineBuffer.size();

    if (position < max)
      ++position;
    else
      position = max;
  }

  void editor::cursor::end () {
    position = editor.lineBuffer.size();
  }

#pragma mark linenoise::editor::history

  void editor::history::add (const std::string &) {

  }

  void editor::history::load (const std::string &) {

  }

  void editor::history::save (const std::string &) {

  }

#pragma mark linenoise::editor

  void editor::refresh () {
    int fd = term.outputFileDescriptor;

    size_t plen = prompt.size();
    size_t pos(cursor.position);

    size_t availableColumns(term.columns() - prompt.size());
    size_t offset(0);

    while ((pos - offset) >= availableColumns) {
      offset++;
    }

    std::stringstream ab;

    ab << "\r" << prompt << lineBuffer.substr(offset, availableColumns) << "\x1b[0K" << "\r\x1b[" << pos + plen - offset << "C";

    const char *abuf = ab.str().c_str();
    write(fd, abuf, strlen(abuf));
  }

  void editor::insert (char c) {
    lineBuffer.insert(cursor.position++, 1, c);
  }

  void editor::kill (bool line) {
    if (line)
      cursor.position = 0;

    if (cursor.position == lineBuffer.length())
      return;

    killBuffer = lineBuffer.substr(cursor.position);
    lineBuffer.erase(cursor.position);
  }

  void editor::yank () {
    lineBuffer.insert(cursor.position, killBuffer);
    cursor.position += killBuffer.length();
  }

  void editor::swap () {
    if (cursor.position != 0) {
      if (cursor.position == lineBuffer.length())
        --cursor.position;

      char c(lineBuffer[cursor.position - 1]);
      deleteBack();
      ++cursor.position;
      insert(c);
    }
  }

  void editor::deleteBack () {
    if (cursor.position > 0) {
      lineBuffer.erase(--cursor.position, 1);
    }
  }

  void editor::deleteForward () {
    lineBuffer.erase(cursor.position, 1);
  }

  void editor::deleteWord () {
    size_t old_pos(cursor.position);

    while (cursor.position > 0 && lineBuffer[cursor.position - 1] == ' ')
      --cursor.position;

    while (cursor.position > 0 && lineBuffer[cursor.position - 1] != ' ')
      --cursor.position;

    lineBuffer.erase(cursor.position, old_pos - cursor.position);
  }

  void editor::historyNext () {

  }

  void editor::historyPrev () {

  }

  bool editor::edit (int stdin_fd, int stdout_fd) {
    lineBuffer = "";
    cursor.position = 0;
    hCursor = -1;

    size_t len(0);

    std::cout << prompt << std::flush;

    while (1) {
      char c;
      ssize_t nread;
      char seq[3];
      len = lineBuffer.length();

      nread = read(term.inputFileDescriptor,&c,1);

      switch ((keyAction)c) {
        case keyAction::ENTER:
          history.add(lineBuffer);
          return true;
        case keyAction::BACKSPACE:
        case keyAction::CTRL_H:
          deleteBack();
          break;
        case keyAction::CTRL_C:
          return false;
        case keyAction::CTRL_D:
          if (len > 0) {
            deleteForward();
          } else {
            done = true;
            return false;
          }
          break;
        case keyAction::CTRL_T:
          swap();
          break;
        case keyAction::CTRL_B:
          cursor.left();
          break;
        case keyAction::CTRL_F:
          cursor.right();
          break;
        case keyAction::CTRL_P:
          historyPrev();
          break;
        case keyAction::CTRL_N:
          historyNext();
          break;
        case keyAction::ESC:
          /* Read the next two bytes representing the escape sequence.
           * Use two calls to handle slow terminals returning the two
           * chars at different times. */
          if (read(term.inputFileDescriptor,seq,1) == -1) break;
          if (read(term.inputFileDescriptor,seq+1,1) == -1) break;

          if (seq[0] == '[') {
            if (seq[1] >= '0' && seq[1] <= '9') {
              if (read(term.inputFileDescriptor,seq+2,1) == -1) break;
              if (seq[2] == '~') {
                switch(seq[1]) {
                  case '3':
                    deleteForward();
                    break;
                }
              }
            } else {
              switch(seq[1]) {
                case 'A':
                  historyPrev();
                  break;
                case 'B':
                  historyNext();
                  break;
                case 'C':
                  cursor.right();
                  break;
                case 'D':
                  cursor.left();
                  break;
                case 'H':
                  cursor.home();
                  break;
                case 'F':
                  cursor.end();
                  break;
              }
            }
          } else if (seq[0] == 'O') {
            switch(seq[1]) {
              case 'H':
                cursor.home();
                break;
              case 'F':
                cursor.end();
                break;
            }
          }
          break;
        default:
          insert(c);
          break;
        case keyAction::CTRL_U:
          kill(true);
          break;
        case keyAction::CTRL_K:
          kill(false);
          break;
        case keyAction::CTRL_Y:
          yank();
          break;
        case keyAction::CTRL_A:
          cursor.home();
          break;
        case keyAction::CTRL_E:
          cursor.end();
          break;
        case keyAction::CTRL_L:
          term.clearScreen();
          break;
        case keyAction::CTRL_W:
          deleteWord();
          break;
      }

      refresh();
    }
  }

  std::string editor::getline () {
    if (term.supported() && isatty(term.inputFileDescriptor)) {
      term.enableRawMode(term.inputFileDescriptor);

      bool output(edit(term.inputFileDescriptor, term.outputFileDescriptor));

      term.disableRawMode(term.inputFileDescriptor);

      std::cout << std::endl;

      return output?lineBuffer:"";
    } else {
      std::cout << prompt << std::flush;

      std::string retval;

      std::getline(std::cin, retval);

      return retval;
    }
  }

}
\$\endgroup\$
5
\$\begingroup\$

I don't have the time to do a completely thorough review at the moment, but here are some things that I noticed that may help you improve your code.

Don't use #import

Although it is supported by some compilers, code which is intended to be reused should avoid non-standard extensions. In this case, it's also not really needed because all of the files except for linenoise.h already contain an include guard, and it's a simple matter to add one to linenoise.h. In a similar vein, it would probably make sense to omit the #pragmas as well. They are inherently compiler-specific. If you feel you must use them, wrap them in guards to hide them from compilers that don't support them. (Yes, compilers will ignore #pragmas they don't support but this often generates a warning.)

Don't reuse the same name for conflicting things

In the existing code, we have these lines of code:

  class editor {
    enum class keyAction;

    class cursor {
      friend class editor;

      const editor &editor;

I see a few problems with that. First, the word class isn't really need in the friend declaration. It's implicit because editor completely defines a type. However, the more serious problem is with the last line because it changes what editor means. On the line above, it referred to a class type. Now it's a const reference to a class of editor type. While compilers may or may not complain about this, users certainly will. One simple way to avoid this would be to use a common convention such as using Editor to name the class and editor to signify an instance of that class.

Eliminate unused variables

Both the parameters stdin_fd and stdout_fd are unused within the edit() routine. Also, the nread variable is set but never used.

Initialize variables before acting on them

In the edit function, the variable c is part of the switch statment, but it hasn't been initialized to any particular value the first time through.

Avoid C-style casts

If you're writing in C++, use C++ style casts rather than C-style casts. They're safer and more explicit to the reader of your code.

Use C-style includes

Instead of including stdio.h you should instead use #include <cstdio>. The difference is in namespaces as you can read about in this question.

\$\endgroup\$
5
  • \$\begingroup\$ Wrap #pragma in a macro or wrap them in guards? :p \$\endgroup\$ – Morwenn Dec 20 '14 at 17:36
  • \$\begingroup\$ In guards of course! Thanks for catching that. \$\endgroup\$ – Edward Dec 20 '14 at 17:39
  • 1
    \$\begingroup\$ It may be possible to put a pragma in a macro though if _Pragma is used instead of #pragma. At least I think it is possible. \$\endgroup\$ – Morwenn Dec 20 '14 at 17:40
  • 1
    \$\begingroup\$ It might be possible, but it's probably not the best way to do it. \$\endgroup\$ – Edward Dec 20 '14 at 17:43
  • 1
    \$\begingroup\$ You're right. Moreover, _Pragma actually takes a parenthesized string literal which gets messy when the pragma already takes a string literal (GCC diagnostic for example). \$\endgroup\$ – Morwenn Dec 21 '14 at 11:22

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.