Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A collection of proposed enhancements #28

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

slcasner
Copy link
Contributor

@slcasner slcasner commented Dec 16, 2017

I have added several enhancements to microrl as we are using it in the Open Vehicle Monitoring System project. I have separated those enhancements into separate commits in my repository here and arranged them with the ones least likely to raise objections applied first.

The trailing whitespace gets removed by editing steps I have done for
other changes, so I wanted to commit this change by itself first to
avoid making the other changes less clear.
Instead of separate memset calls and assignments to initialize various
members of the microrl_t struct to zero, just clear the entire struct
first and then set the nonzero values.
1. Add a 'void* userdata' member to microrl_t that can be used by
applications to store a C++ object pointer or other context info.

2. Pass the pointer to microrl_t in all callbacks so that the
operations can be specific to a particular instance of microrl.

3. Enclose the definitions in microrl.h in a conditional 'extern "C"'
block to allow those definitions to be referenced from C++ files.
This is a change that was missed in copying over to this repository in
the previous commit 7334fbd.
Change the example code to include the microrl_t pointer as the first
argument of all the callbacks as was added to the microrl library in
commit 7334fbd.

Note: The changes in avr_misc have not been tested since I don't have
a build environment.
Typing multiple Ctrl-D while on an empty command line would crash.
This was missed in merging changes from another repository.
When used over a network connection, each print() call in microrl may
be transmitted as a separate packet, so the many small print
operations make output slow.  This commit optimizes some common cases
to avoid unnecessary prints and combines others into a buffer to be
printed together.

- When adding characters to the end of a command, now just the single
  character is output.  This avoids cursor jumping.  Similarly, when
  backspacing at the end of the line, just the simple sequence
  back-one, space, back-one is sufficient.

- The biggest number of back-to-back print operations occurred in
  terminal_print_line() where each character was printed separately.
  Now the characters are packed into a temporary buffer first so they
  can be printed in one or a few operations, depending upon the
  command length and buffer size.  Positioning sequences are also
  consolidated into the buffer.

- Some calls to terminal_reset_cursor() that were changed to call
  terminal_move_cursor() instead since the latter outputs fewer
  characters and avoids extra jumping of the cursor on slow lines.
  The code to reset the cursor position was then incorporated into
  terminal_print_line() where those positioning operations can be
  consolidated into a print buffer.  Since the reset is not always
  necessary, an argument controls whether it is done.

- A new define _USE_CARRIAGE_RETURN controls using a carriate return
  to reposition the cursor to the left margin rather than moving left
  by a large number.  This takes fewer characters.  This optimization
  can be turned off if the terminal simulates receiving a linefeed
  when it receives the carriage return.

- In terminal_print_line(), the delete-to-end-of-line escape sequence
  is moved to be output after all of the command text.  This avoids a
  flash of the text going away and then being repainted.

- The code to create repositioning sequences was extracted into a
  separate function generate_move_cursor() so it can be shared between
  terminal_move_cursor() and terminal_print_line().

- After reorganizing the code to generate repositioning strings there
  was only one call to u16bit_to_str(), so it was merged inline.

- strcpy() calls of just a few characters are replaced by depositing
  the characters individually.  This removes a dependency and may
  use fewer instructions.

- To allow ^U to backspace across all of the characters to the left of
  the curso in one motion, microrl_backspace() now takes a parameter
  for the number of spaces to move rather than always moving one.
@antonblanchard
Copy link

Thanks @slcasner! I've pulled in your fixes and one from @tomlogic for a project of mine. @Helius any chance we a few of the pull requests closed out?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants