Patch for shell commands (rm & clear) and small cleanup

classic Classic list List threaded Threaded
4 messages Options
raman raman
Reply | Threaded
Open this post in threaded view
|

Patch for shell commands (rm & clear) and small cleanup


Dear eLua community,

This patch fixes the following:

1. Add print description for the new mkdir in shell_help.
    (this is currently missing) and cleans up an unused
    variable mkdir.

2. Add rm command (please look into patch).

3. Added a 'clear' command.

Please let me know if the patch is missing something.

Best,
Raman

elua.shcmd.patch
Martin Guy Martin Guy
Reply | Threaded
Open this post in threaded view
|

Re: Patch for shell commands (rm & clear) and small cleanup

On 26 September 2012 13:16, raman <[hidden email]> wrote:

>
> Dear eLua community,
>
> This patch fixes the following:
>
> 1. Add print description for the new mkdir in shell_help.
>     (this is currently missing) and cleans up an unused
>     variable mkdir.
>
> 2. Add rm command (please look into patch).
>
> 3. Added a 'clear' command.
>
> Please let me know if the patch is missing something.

Hi
  The usual practice is to put one functional change in each patch,
not three unrelated ones (well, two vaguely related ones and one
random one).

In particular, the clear command has nothing to do with MMCFS
operations, and has the unfortunate side effect of making the TERM
module compulsory if you include the shell, which can be important for
devices with small code size.
It also seems out of place in a shell where all the other operations
are filesystem-related.

Cheers

    M
_______________________________________________
eLua-dev mailing list
[hidden email]
https://lists.berlios.de/mailman/listinfo/elua-dev
raman raman
Reply | Threaded
Open this post in threaded view
|

Re: Patch for shell commands (rm & clear) and small cleanup


Dear Martin,

Sorry for the late reply.

>  The usual practice is to put one functional change in each patch,
> not three unrelated ones (well, two vaguely related ones and one
> random one).

Sorry about this. I'll keep this in mind for the future patches.

> In particular, the clear command has nothing to do with MMCFS
> operations, and has the unfortunate side effect of making the TERM
> module compulsory if you include the shell, which can be important for
> devices with small code size.
> It also seems out of place in a shell where all the other operations
> are filesystem-related.

Agreed. Thanks for your suggestions.

The rm command, I thought I'll implement as a response to this post:
http://elua-development.2368040.n2.nabble.com/added-a-rm-to-eLua-shell-td7557508.html

Do I submit a separate patch for this?

Best,
Raman
BogdanM BogdanM
Reply | Threaded
Open this post in threaded view
|

Re: Patch for shell commands (rm & clear) and small cleanup

Hi,

On Mon, Oct 15, 2012 at 9:14 PM, raman <[hidden email]> wrote:

>
> Dear Martin,
>
> Sorry for the late reply.
>
>>  The usual practice is to put one functional change in each patch,
>> not three unrelated ones (well, two vaguely related ones and one
>> random one).
>
> Sorry about this. I'll keep this in mind for the future patches.
>
>> In particular, the clear command has nothing to do with MMCFS
>> operations, and has the unfortunate side effect of making the TERM
>> module compulsory if you include the shell, which can be important for
>> devices with small code size.
>> It also seems out of place in a shell where all the other operations
>> are filesystem-related.
>
> Agreed. Thanks for your suggestions.
>
> The rm command, I thought I'll implement as a response to this post:
> http://elua-development.2368040.n2.nabble.com/added-a-rm-to-eLua-shell-td7557508.html

A 'rm' command is already implemented on the "shellfs" branch (on
github), so I'm guessing we don't really need another one :)

Best,
Bogdan

>
> Do I submit a separate patch for this?
>
> Best,
> Raman
>
>
>
>
> --
> View this message in context: http://elua-development.2368040.n2.nabble.com/Patch-for-shell-commands-rm-clear-and-small-cleanup-tp7577691p7577715.html
> Sent from the eLua Development mailing list archive at Nabble.com.
> _______________________________________________
> eLua-dev mailing list
> [hidden email]
> https://lists.berlios.de/mailman/listinfo/elua-dev
_______________________________________________
eLua-dev mailing list
[hidden email]
https://lists.berlios.de/mailman/listinfo/elua-dev