Skip to content

shell: "select" command suddenly gone #10769

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

Closed
pfalcon opened this issue Oct 22, 2018 · 14 comments
Closed

shell: "select" command suddenly gone #10769

pfalcon opened this issue Oct 22, 2018 · 14 comments
Assignees
Labels
area: Shell Shell subsystem Release Notes To be mentioned in the release notes RFC Request For Comments: want input from the community
Milestone

Comments

@pfalcon
Copy link
Collaborator

pfalcon commented Oct 22, 2018

Following up on the quick discussion on IRC.

So, a user which used both old and new shell would immediately notice the lack of "select" command in the new one. On IRC, I was told that it's not needed, and one can access all commands directly. I don't what was meant by that, but my interpretation is obvious - it would mean that knowing that there's "stats" command in "net" shell module, I can type "stats" and it would get executed.

Of course, it cannot work like that, because there could be "bt stats", etc. commands. So, I proceeded to check, and of course, "stats" doesn't work:

uart:~$ stats
stats: command not found

Instead, I need to type:

uart:~$ net stats 

Interface 0x00424a20 (Ethernet) [0]
===================================

Of course, it's alleviated by completion, but:

  1. Completion was there in the old shell too.
  2. It was possible to use "net stats" in the old shell too.
  3. But besides that, there was also "select <module>" command which allowed get rid of typing "net" altogether!

Of course, with possibility no.3 provided, users enjoyed it. And of course, they noticed the regression with the new shell, and submit tickets asking what's up.

@jakub-uC
Copy link
Collaborator

jakub-uC commented Oct 22, 2018

@carlescufi @nashif @pfalcon
New shell has different architecture comparing to the old one. It was highlighted since the very beginning that it will not have namespaces. Of course it would be feasable to implement select command but in my opinion it will have follwoing drawbacks:

  • it will not bring realy new functionality
  • it will be extra code that must be maintained
  • memory footprint will increase
  • it might confuse the user when subcommand and namespace have the same syntax - what shall be in fact selected?

I am not 100% against but at least I would like to have fair discussion about that. New shell is widely announced hence I expect users are aware about possible changes and losing backward compatibility.

@pfalcon
Copy link
Collaborator Author

pfalcon commented Oct 22, 2018

So, unlike some other things (e.g. #10766) I also not 100% "want it back". I'm just taking a role of an end user (and I am such regarding the use of the shell), and trying to understand how it happened that new, improved shell was designed in a way to not support previous shell features. And then to collect feedback from other people whether they miss it. If I'm the only one, well, I guess I can re-learn.

@jukkar
Copy link
Member

jukkar commented Oct 23, 2018

IMHO we do not need select command, personally I never used it earlier anyway.

@pfalcon
Copy link
Collaborator Author

pfalcon commented Oct 29, 2018

personally I never used it earlier anyway.

But people apparently did, see e.g. 0b8fde3 . Quoting the commit message:

This driver has been tested with stm32l4 disco iot board
(disco_l475_iot1) and the wifi sample:

$ select wifi
$ scan
$ connect "CISCO" 5 password
$ select net
$ tcp connect 192.168.1.21 4242
$ tcp send HelloWorld!

@nashif nashif added the Release Notes To be mentioned in the release notes label Oct 30, 2018
@nashif nashif added this to the v1.14.0 milestone Oct 30, 2018
@nashif
Copy link
Member

nashif commented Oct 30, 2018

This is no regression, when you completely change how the subsystem works, to the better, it happens. I have not seen anyone complaining about this being missing or being broken because of that. Very nice to be speaking for the "users", but I did not hear any users complaining about that. This will will need to be documented in the release notes and should be enough IMO. Let's move on.

If there is a real bug here, please open one with specific details.

@nashif nashif closed this as completed Oct 30, 2018
@nordic-krch
Copy link
Collaborator

Fyi, during hackathon @jarz-nordic was playing around making select command which goes one step into command tree. The trick there was that when you are switching to commands 'branch' you don't have unselect command. For that he used metakey ctrl+x. Such approach emulates starting an application and quitting it. Not sure where that ends and if he plans to do PR since it's rather nice to have feature.

@pfalcon
Copy link
Collaborator Author

pfalcon commented Oct 30, 2018

@nashif: This ticket is open for community discussion. Please don't close it single-headedly, unless you want to make (repeated) precedents of how tickets are handled in the Zephyr project. Obviously, you won't "hear" complaints, if there's a motion to shut up any such discussion.

Otherwise, thanks for your opinion. I fully agree that this matter should be finalized until 1.14 release, so it's useful to keep it open until then.

@pfalcon pfalcon reopened this Oct 30, 2018
@pfalcon
Copy link
Collaborator Author

pfalcon commented Oct 30, 2018

during hackathon @jarz-nordic was playing around making select command which goes one step into command tree

I do hope that nobody spends implementation effort on this ticket until it's proven as required. I also do hope that it remains a place of discussion, and ultimately, an exercise of change control for all of us.

@pfalcon pfalcon added the RFC Request For Comments: want input from the community label Oct 30, 2018
@nashif
Copy link
Member

nashif commented Nov 2, 2018

@pfalcon then please remove the "Regression" from the title. This is not a regression.

@nashif
Copy link
Member

nashif commented Nov 2, 2018

@pfalcon

Please don't close it single-headedly

I still need to find the button that supports multi-headed closing.

@pfalcon pfalcon changed the title shell: Regression: "select" command is gone shell: "select" command suddenly gone Nov 2, 2018
@jakub-uC
Copy link
Collaborator

jakub-uC commented Feb 6, 2019

@pfalcon : I am working on select command. I have following assumptions in my mind:

  1. It is allowed to select a command that:
  • is static (syntax/handler cannot change in runtime)
  • has subcommands
  1. You can always select command that is deeper in commands tree if it fulfills 1)

  2. When command is successfully selected, prompt will change to selected command syntax + $ or > (TBD).

  3. To unselect command you can either type select with no arguments or select <no valid argument>

  4. Unselect will always bring you to command's ROOT_LEVEL (you cannot recursively unselect).

@pfalcon
Copy link
Collaborator Author

pfalcon commented Feb 11, 2019

@jarz-nordic

I am working on select command.

@jarz-nordic: Thanks for bringing this up. I actually wanted to post my impressions of "3 months without select". In short, I still miss it, even if mostly subconsciously. If I don't use the shell for 3-4, next session I either start with automatically typing "select", or with typing "iface", "ping", etc., and getting error.

Now after posting that, my intention was to close this ticket, because it didn't generate much interest, and as I told a few times, while I consider such matters to be a topic for discussion, I don't consider that someone should spend effort on such obviously subjective matters only because I want.

So, please think again if it's good investment of your time. If you implement it, I will use and will appreciate it. The way I would image it to be implement is to store select's argument in some buffer, then prepended to each command typed later (max line length should be taken into account). Except of course not everything can be an arg of "select", but a "subcategory command" like "net".

All in all, the above is pretty trivial, and you would certainly know better. But surely let's make it KISS.

When command is successfully selected, prompt will change to selected command syntax + $ or > (TBD).

I like this. All other points also makes sense.

Thanks.

@jakub-uC
Copy link
Collaborator

@pfalcon : I am quite close to PR so please expect it shortly.

@jakub-uC
Copy link
Collaborator

According to comments in #13425 this issue can be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Shell Shell subsystem Release Notes To be mentioned in the release notes RFC Request For Comments: want input from the community
Projects
None yet
Development

No branches or pull requests

6 participants