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

nshlib/prompt: add multiple sources support at runtime #2300

Merged
merged 1 commit into from
Feb 25, 2024

Conversation

yf13
Copy link
Contributor

@yf13 yf13 commented Feb 21, 2024

Summary

Currently NSH prompt string is determined at build time, this is inflexible in some AMP cases where the same NSH binary runs on different AMP nodes as the same prompt string is showed everywhere. Yet another issue is that empty NSH_RPOMPT_STRING config leads to no prompt at all, making people questioning if the system is stalled.

This patch attempts to solve these issues by adding ordered runtime fallbacks for empty NSH_PROMPT_STRING case from sources like environment variable (default to PS1) and hostname, both suffixed with >. If all sources are empty, the non-empty suffix string will be used. The patch has little impacts on for existing use cases with non-empty NSH_PROMPT_STRING .

Changes in nshlib/ folder:

  • Kconfig: add NSH_PROMPT_MAX and NSH_PROMPT_ENV configs
  • nsh.h: adjust g_nshprompt defs, add nsh_update_prompt
  • nsh_parse.c move g_nshpromt to nsh_prompt.c
  • nsh_init.c revise with nsh_update_prompt
  • Makefile add nsh_prompt.c
  • CMakeLists.txt add nsh_prompt.c

New additions in nshlib/

  • nsh_prompt.c contains prompt related data and methods.

Impact

NSH app

Testing

Checked with CanMV230

Limits

All sources are plaintext, there is no shell escaping, terminfo escaping, command embedding etc.

yf13 added a commit to yf13/nuttx that referenced this pull request Feb 22, 2024
This patch fixes CI blocking warnings of unused `fs` variables in hostfs.c.
It blocks apache/nuttx-apps#2300.

Signed-off-by: Yanfeng Liu <yfliu2008@qq.com>
@xiaoxiang781216
Copy link
Contributor

but you can set CONFIG_NSH_PROMPT_STRING to different string in each defconfig.

@yf13
Copy link
Contributor Author

yf13 commented Feb 22, 2024

@xiaoxiang781216 using NSH_PROMPT_STRING config can't solve the issue as the master and remote node shares the same NSH binary, thus both master and remote consoles show the same prompt string, this confuses NSH console users.

@xiaoxiang781216
Copy link
Contributor

@xiaoxiang781216 using NSH_PROMPT_STRING config can't solve the issue as the master and remote node shares the same NSH binary, thus both master and remote consoles show the same prompt string.

If so, let's use the approach like bash: https://www.cyberciti.biz/tips/howto-linux-unix-bash-shell-setup-prompt.html

@yf13
Copy link
Contributor Author

yf13 commented Feb 22, 2024

@xiaoxiang781216 did you mean that NSH already supports special variables PS1? I tried using set PS1 aaa in NSH session, I can see PS1=aaa via env command, but the prompt string doesn't change at all. Did I miss anything?

Or did you mean that we should add PS1 alike feature to NSH? that may need more time/effort as I am still unfamilar with NSH.

@xiaoxiang781216
Copy link
Contributor

@xiaoxiang781216 did you mean that NSH already supports special variables PS1? I tried using set PS1 aaa in NSH session, I can see PS1=aaa via env command, but the prompt string doesn't change at all. Did I miss anything?

NSH doesn't support PS1 yet.

Or did you mean that we should add PS1 alike feature to NSH? that may need more time/effort as I am still unfamilar with NSH.

it's simple, something like this:

FAR const char *nsh_getprompt(void)
{
  FAR const char *ps1 = getenv("PS1");
  return ps1 ? ps1 : g_nshprompt;
}

@yf13
Copy link
Contributor Author

yf13 commented Feb 22, 2024

@xiaoxiang781216 we may need check how readline works. it has an internal reference of g_nshprompt.

@yf13 yf13 changed the title nshlib/prompt: allow runtime fallback to hostname nshlib/prompt: add runtime prompt string determination support Feb 22, 2024
@yf13 yf13 changed the title nshlib/prompt: add runtime prompt string determination support nshlib/prompt: add runtime prompt string determination Feb 22, 2024
@yf13
Copy link
Contributor Author

yf13 commented Feb 22, 2024

@xiaoxiang781216 please see if this version works or not? it has PS1 support.

@yf13 yf13 force-pushed the use-hostname-as-nsh-prompt branch 3 times, most recently from be12c71 to 3a9c08e Compare February 22, 2024 21:29
@yf13 yf13 changed the title nshlib/prompt: add runtime prompt string determination nshlib/prompt: add empty prompt string fallbacks Feb 22, 2024
nshlib/Kconfig Outdated Show resolved Hide resolved
nshlib/Kconfig Show resolved Hide resolved
nshlib/nsh.h Outdated Show resolved Hide resolved
nshlib/nsh.h Outdated Show resolved Hide resolved
nshlib/nsh_prompt.c Outdated Show resolved Hide resolved
nshlib/nsh_prompt.c Outdated Show resolved Hide resolved
nshlib/nsh_prompt.c Outdated Show resolved Hide resolved
nshlib/nsh_prompt.c Outdated Show resolved Hide resolved
nshlib/nsh_prompt.c Outdated Show resolved Hide resolved
nshlib/nsh_prompt.c Outdated Show resolved Hide resolved
@yf13 yf13 force-pushed the use-hostname-as-nsh-prompt branch 7 times, most recently from 8acf84d to 945a347 Compare February 24, 2024 08:09
nshlib/Kconfig Outdated Show resolved Hide resolved
nshlib/nsh_prompt.c Outdated Show resolved Hide resolved
nshlib/nsh_prompt.c Outdated Show resolved Hide resolved
nshlib/nsh_init.c Show resolved Hide resolved
nshlib/nsh_prompt.c Outdated Show resolved Hide resolved
nshlib/nsh_prompt.c Outdated Show resolved Hide resolved
nshlib/nsh_prompt.c Outdated Show resolved Hide resolved
nshlib/nsh_prompt.c Outdated Show resolved Hide resolved
nshlib/nsh_init.c Outdated Show resolved Hide resolved
nshlib/nsh_prompt.c Outdated Show resolved Hide resolved
nshlib/nsh_prompt.c Outdated Show resolved Hide resolved
nshlib/nsh_prompt.c Outdated Show resolved Hide resolved
nshlib/nsh_prompt.c Outdated Show resolved Hide resolved
nshlib/nsh_prompt.c Outdated Show resolved Hide resolved
nshlib/Kconfig Outdated Show resolved Hide resolved
nshlib/Kconfig Outdated Show resolved Hide resolved
nshlib/nsh_prompt.c Outdated Show resolved Hide resolved
nshlib/nsh_prompt.c Outdated Show resolved Hide resolved
@yf13 yf13 force-pushed the use-hostname-as-nsh-prompt branch 4 times, most recently from 65eca55 to a026515 Compare February 24, 2024 16:05
nshlib/nsh_prompt.c Outdated Show resolved Hide resolved
nshlib/Kconfig Outdated Show resolved Hide resolved
nshlib/nsh_prompt.c Outdated Show resolved Hide resolved
nshlib/nsh_prompt.c Outdated Show resolved Hide resolved
nshlib/nsh_prompt.c Outdated Show resolved Hide resolved
nshlib/nsh_prompt.c Outdated Show resolved Hide resolved
nshlib/nsh_prompt.c Outdated Show resolved Hide resolved
nshlib/nsh_prompt.c Outdated Show resolved Hide resolved
@yf13 yf13 force-pushed the use-hostname-as-nsh-prompt branch 2 times, most recently from fd75c3a to cd802bf Compare February 25, 2024 02:17
nshlib/nsh_prompt.c Outdated Show resolved Hide resolved
nshlib/nsh_prompt.c Outdated Show resolved Hide resolved
nshlib/nsh_prompt.c Outdated Show resolved Hide resolved
nshlib/nsh_prompt.c Outdated Show resolved Hide resolved
nshlib/nsh_prompt.c Outdated Show resolved Hide resolved
nshlib/nsh_prompt.c Outdated Show resolved Hide resolved
@yf13 yf13 force-pushed the use-hostname-as-nsh-prompt branch 3 times, most recently from 0a1cdbe to 187726a Compare February 25, 2024 04:00
Currently NSH prompt is defined at build time, thus improper for AMP cases
where the same NSH binary is used on different nodes as the same NSH prompt
shows on all nodes.

This patch attempts to support runtime prompt string population from ordered
sources:
  - the environment variable defined by NSH_PROMPT_ENV plus suffix
  - the NSH_PROMPT_STRING
  - the HOSTNAME plus suffix
The suffix is defined by NSH_PROMPT_SUFFIX so that to clearly separate the
command inputs.

Changes in `nshlib/`

- Kconfig:       add configs NSH_PROMPT_MAX/ENV/SUFFIX etc
- nsh.h:         adjust g_nshprompt defs, add nsh_update_prompt
- nsh_parse.c    relocate g_nshpromt to nsh_prompt.c
- nsh_init.c     revise to use nsh_update_prompt once
- nsh_session.c  revise to use methods in nsh_prompt.c
- Makefile       add nsh_prompt.c
- CMakeLists.txt add nsh_prompt.c

New additions in `nshlib/`

- nsh_prompt.c   prompt related data structures and methods.

Signed-off-by: Yanfeng Liu <yfliu2008@qq.com>
@xiaoxiang781216 xiaoxiang781216 merged commit 31908b3 into apache:master Feb 25, 2024
26 checks passed
@yf13 yf13 changed the title nshlib/prompt: add empty prompt string fallbacks nshlib/prompt: add multiple source support for prompt string Feb 27, 2024
@yf13 yf13 changed the title nshlib/prompt: add multiple source support for prompt string nshlib/prompt: add multiple sources support at runtime Feb 27, 2024
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.

None yet

2 participants