[spp] [PATCH 1/2] spp: update to improve usability

[spp] [PATCH 1/2] spp: update to improve usability

[ogawa.yasufumi]

From: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>

SPP controller executes previous command if user just types enter in
command line. It decreases usability and might cause a wrong operation.
In addition, users cannot use empty lines in config files loaded from
`playback` command. It also decreases maintainability.

For maintainability, it is another problem for SPP controller does not
accept comment line from CLI (only able to use only in config file).
User cannot record comment in 'record' command for the reason.

This patch includes following updates.

* Override emptyline() of Shell class to not repeat previous command.

* Override default() to accept comment line. To find the comment, add
  is_comment_line() which support python and C style comment.

* Remove lower() for user's input to avoid unexpected behaviour.

Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
 src/spp.py | 37 +++++++++++++++++++++++++++++++++----
 1 file changed, 33 insertions(+), 4 deletions(-)

diff --git a/src/spp.py b/src/spp.py
index 3796670..84f1fa7 100755
--- a/src/spp.py
+++ b/src/spp.py
@@ -346,7 +346,7 @@ def clean_sec_cmd(cmdstr):
     return res
 
 
-class Shell(cmd.Cmd):
+class Shell(cmd.Cmd, object):
     """SPP command prompt"""
 
     intro = 'Welcome to the spp.   Type help or ? to list commands.\n'
@@ -365,6 +365,36 @@ class Shell(cmd.Cmd):
     SEC_SUBCMDS = ['vhost', 'ring', 'pcap', 'nullpmd']
     BYE_CMDS = ['sec', 'all']
 
+    def is_comment_line(self, line):
+        input_line = line.strip()
+        if len(input_line) > 0:
+            if (input_line[0] == '#') or (input_line[0:2] == '//'):
+                return True
+            else:
+                return False
+
+    def default(self, line):
+        """Define defualt behaviour
+
+        If user input is commend styled, controller simply echo as a comment.
+        Supported styles are
+          - python ('#')
+          - C ('//')
+        """
+
+        if self.is_comment_line(line):
+            print("%s" % line.strip())
+        else:
+            super(Shell, self).default(line)
+
+    def emptyline(self):
+        """Do nothin for empty input
+
+        It override Cmd.emptyline() which runs previous input as default
+        to do nothing.
+        """
+        pass
+
     def close_all_secondary(self):
         """Exit all secondary processes"""
 
@@ -626,8 +656,8 @@ class Shell(cmd.Cmd):
                 with open(fname) as recorded_file:
                     lines = []
                     for line in recorded_file:
-                        if line.strip().startswith("#"):
-                            continue
+                        if not self.is_comment_line(line):
+                            lines.append("# %s" % line)
                         lines.append(line)
                     self.cmdqueue.extend(lines)
                     self.response(self.CMD_OK, "playback")
@@ -637,7 +667,6 @@ class Shell(cmd.Cmd):
                 self.response(self.CMD_NG, message)
 
     def precmd(self, line):
-        line = line.lower()
         if self.recorded_file:
             if not (('playback' in line) or ('bye' in line)):
                 print(line, file=self.recorded_file)
-- 
2.7.4

[spp] [PATCH 2/2] spp_vf: update to improve usability

[ogawa.yasufumi]

From: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>

This update is same as previous patch for spp.py.

* Override emptyline() of Shell class to not repeat previous command.

* Override default() to accept comment line. To find the comment, add
  is_comment_line() which support python and C style comment.

* Remove lower() for user's input to avoid unexpected behaviour.

Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
 src/spp_vf.py | 37 +++++++++++++++++++++++++++++++++----
 1 file changed, 33 insertions(+), 4 deletions(-)

diff --git a/src/spp_vf.py b/src/spp_vf.py
index 0d706ee..4325c28 100755
--- a/src/spp_vf.py
+++ b/src/spp_vf.py
@@ -301,7 +301,7 @@ def check_sec_cmds(cmds):
     return valid
 
 
-class Shell(cmd.Cmd):
+class Shell(cmd.Cmd, object):
     """SPP command prompt"""
 
     intro = 'Welcome to the spp.   Type help or ? to list commands.\n'
@@ -311,6 +311,36 @@ class Shell(cmd.Cmd):
     COMMANDS = ['status', 'add', 'patch', 'ring', 'vhost',
                 'reset', 'exit', 'forward', 'stop', 'clear']
 
+    def is_comment_line(self, line):
+        input_line = line.strip()
+        if len(input_line) > 0:
+            if (input_line[0] == '#') or (input_line[0:2] == '//'):
+                return True
+            else:
+                return False
+
+    def default(self, line):
+        """Define defualt behaviour
+
+        If user input is commend styled, controller simply echo as a comment.
+        Supported styles are
+          - python ('#')
+          - C ('//')
+        """
+
+        if self.is_comment_line(line):
+            print("%s" % line.strip())
+        else:
+            super(Shell, self).default(line)
+
+    def emptyline(self):
+        """Do nothin for empty input
+
+        It override Cmd.emptyline() which runs previous input as default
+        to do nothing.
+        """
+        pass
+
     def complete_pri(self, text, line, begidx, endidx):
         """Completion for primary process commands"""
 
@@ -375,15 +405,14 @@ class Shell(cmd.Cmd):
             with open(arg) as recorded_file:
                 lines = []
                 for line in recorded_file:
-                    if line.strip().startswith("#"):
-                        continue
+                    if not self.is_comment_line(line):
+                        lines.append("# %s" % line)
                     lines.append(line)
                 self.cmdqueue.extend(lines)
         except IOError:
             print("Error: File does not exist.")
 
     def precmd(self, line):
-        line = line.lower()
         if self.recorded_file and 'playback' not in line:
             print(line, file=self.recorded_file)
         return line
-- 
2.7.4

Re: [spp] [PATCH 1/2] spp: update to improve usability

[Ferruh Yigit]

On 2/8/2018 3:18 PM, ogawa.yasufumi@lab.ntt.co.jp (ogawa.yasufumi@lab.ntt.co.jp)
wrote:
> From: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
> 
> SPP controller executes previous command if user just types enter in
> command line. It decreases usability and might cause a wrong operation.
> In addition, users cannot use empty lines in config files loaded from
> `playback` command. It also decreases maintainability.
> 
> For maintainability, it is another problem for SPP controller does not
> accept comment line from CLI (only able to use only in config file).
> User cannot record comment in 'record' command for the reason.
> 
> This patch includes following updates.
> 
> * Override emptyline() of Shell class to not repeat previous command.
> 
> * Override default() to accept comment line. To find the comment, add
>   is_comment_line() which support python and C style comment.
> 
> * Remove lower() for user's input to avoid unexpected behaviour.
> 
> Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>

Series applied to dpdk-next-net/master, thanks.

Re: [spp] [PATCH 2/2] spp_vf: update to improve usability

[Ferruh Yigit]

On 2/8/2018 3:18 PM, ogawa.yasufumi@lab.ntt.co.jp (ogawa.yasufumi@lab.ntt.co.jp)
wrote:
> From: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
> 
> This update is same as previous patch for spp.py.
> 
> * Override emptyline() of Shell class to not repeat previous command.
> 
> * Override default() to accept comment line. To find the comment, add
>   is_comment_line() which support python and C style comment.
> 
> * Remove lower() for user's input to avoid unexpected behaviour.
> 
> Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>

Hi Yasufumi,

This is already applied but I am concerned about having two version of python
script, spp.py and spp_vf.py.

Is there a long term plan for them?
Why we are maintaining two copies and they are slightly differentiated now, as
some features only supported in spp_vf.py?

Does it make sense to merge them?

Thanks,
ferruh

Re: [spp] [PATCH 2/2] spp_vf: update to improve usability

[Yasufumi Ogawa]

On 2018/02/22 20:50, Ferruh Yigit wrote:
> On 2/8/2018 3:18 PM, ogawa.yasufumi@lab.ntt.co.jp (ogawa.yasufumi@lab.ntt.co.jp)
> wrote:
>> From: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
>>
>> This update is same as previous patch for spp.py.
>>
>> * Override emptyline() of Shell class to not repeat previous command.
>>
>> * Override default() to accept comment line. To find the comment, add
>>    is_comment_line() which support python and C style comment.
>>
>> * Remove lower() for user's input to avoid unexpected behaviour.
>>
>> Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
> 
> Hi Yasufumi,
> 
> This is already applied but I am concerned about having two version of python
> script, spp.py and spp_vf.py.
> 
> Is there a long term plan for them?
> Why we are maintaining two copies and they are slightly differentiated now, as
> some features only supported in spp_vf.py?
Ferruh,

I think I should move spp_vf features to spp.py. We started to develop 
spp_vf as an experimental project independently, but it is merged SPP repo.

As you mentioned, spp_vf.py has features only for spp_vf, however I 
think it can be moved to spp.py as an extension or plugin to not lose 
usability. I'd like to consider it.

Thanks,
Yasufumi
> 
> Does it make sense to merge them?
> 
> Thanks,
> ferruh
> 
> 


-- 
Yasufumi Ogawa
NTT Network Service Systems Labs