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

New "readline" module for a better shell #2751

Closed
wants to merge 7 commits into from
Closed

New "readline" module for a better shell #2751

wants to merge 7 commits into from

Conversation

janoskut
Copy link

@janoskut janoskut commented Apr 1, 2015

This is a more sophisticated shell implementation, that has colors, cursor-navigation, bash/zsh-like shortcuts and even a history (no tab-completion yet).

The implementation can be integrated as a module via USEMODULE += readline. The dependency to the shell-module is provided.

The shell module can now be configured with USE_SHELL_COLORS=1 to have fancy colors.
The readline module can be configured with SHELL_HISTORY_SIZE=<n> to have a history-buffer that is n-times the buffer size. SHELL_HISTORY_SIZE=0 compiles without the usage of the history, as well as not mentioning that flag at all.

TODOs:
The SHELL_BUFFER_SIZE has a big FIXME written next to it in readline.h. It should be of the same size as the buffer size used for the shell module, but i haven't figured a way out yet to combine them.

@OlegHahm OlegHahm added the Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation label Apr 1, 2015
@miri64 miri64 assigned LudwigKnuepfer and unassigned miri64 Apr 1, 2015
static char cut_buffer[SHELL_BUFFER_SIZE];

// TODO: Introduce definition PROMT_LENGTH (instead of "2")
#define PROMT_LENGTH 2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is meant to be PROMPT_LENGTH

@miri64
Copy link
Member

miri64 commented Apr 2, 2015

To be clear: this is a native only feature right?

@miri64
Copy link
Member

miri64 commented Apr 2, 2015

Because as a far as I know: pyterm provides most of this already.

@miri64 miri64 added the Platform: native Platform: This PR/issue effects the native platform label Apr 2, 2015
@Kijewski
Copy link
Contributor

Kijewski commented Apr 2, 2015

Does RIOT really need to provide terminal features?

@janoskut
Copy link
Author

janoskut commented Apr 2, 2015

@authmillenon No this feature is independent of the port, just as the shell module. Only for the native port, ncurses is needed to use stdio in raw mode.

@Kijewski This is not needed at all, but I found it nice to have. Pyterm is just one of many terminal programs, I'm sure different people have different favours and different use cases may also require different terminals. For that matter, terminal features may be of interest.

@LudwigKnuepfer
Copy link
Member

please add module prefixes to the commits.

@x3ro
Copy link
Contributor

x3ro commented Apr 2, 2015

Haven't had time to look into the "why" yet, but the PR currently bumps the RAM usage on the SAMR21 by 512 bytes when not including the readline module and by 648 bytes when including it.

@LudwigKnuepfer
Copy link
Member

Needs rebase

@janoskut
Copy link
Author

janoskut commented Apr 3, 2015

@LudwigOrtmann I will add prefixes, what is the syntax for it? "sys/readline:"?
@LudwigOrtmann Rebase - what am I supposed to do?
@x3ro I'd like to figure out why that is, can you give me a hint on how I can observe the RAM usages?

@x3ro
Copy link
Contributor

x3ro commented Apr 3, 2015

@noshky if you build for the SAMR with

BOARD=samr21-xpro make clean all

It will display the size of the different sections in the binary at the end. The section you're interested in for RAM usage is bss.

Janos Kutscherauer added 7 commits April 5, 2015 20:55
…tion.

The color output can be enabled by setting USE_SHELL_COLORS=1.
Also added an example of color usage to the shell-test app.
The module contains:
- Recognition of VT102/ANSI compatible control characters.
- Bash/zsh compatible keyboard shortcuts for line edditing.
- A space-efficient history implementation, configurable in size and enablement (SHELL_HISTORY_SIZE=n).
- A shell-command handler to print the READLINE-module keyboard shortcuts.

This shell implementation will depend on the curses/ncurses library for the native port.
…is needed for the shell/readline implementation.

The usage of ncurses is needed to configure the terminal/shell, in which native applications are run.
This adds the usage of the LIBNCURSES and LIBTINFO to the native port.
The ncurses initialization and de-initialization is done in the native-uart0 startup handlers.
To enable the proper restart of native terminal-apps, the ENV variable is stored and re-used upon restart.
The readline-implementation is only used when the module is activated, otherwise the regular implementation is used.
@LudwigKnuepfer
Copy link
Member

@noshky seems you figured this out on your own =)

@OlegHahm OlegHahm assigned x3ro and unassigned LudwigKnuepfer Apr 9, 2015
@miri64
Copy link
Member

miri64 commented May 27, 2015

Found another use-case for this module: iotlab-term doesn't use pyterm either and is would also benefit, I guess. @OlegHahm what do you think?

@OlegHahm
Copy link
Member

You can easily use pyterm with IoT-LAB and the serial-aggregator also uses readline.

@miri64
Copy link
Member

miri64 commented May 27, 2015

How? I'm having problems accessing the history.

@OlegHahm
Copy link
Member

That's probably an ssh issue. Try with #2952

@OlegHahm
Copy link
Member

I think it would be a nice feature for toying around, but not really enhance RIOT handling, but just increase memory usage. Since original contributor seems to have abandoned this anyway, I think we can close this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Platform: native Platform: This PR/issue effects the native platform Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants