Re: [apparmor] [patch] new profile tools - handling of (F)inish

2014-02-24 Thread Christian Boltz 
Hello,

[patch v2, see below]

Am Montag, 27. Januar 2014 schrieb Christian Boltz:
 currently, selecting (F)inish in the new profile tools basically means
 aborting without saving anything. However, we already have Abo(r)t
 for that ;-)
 
 (F)inish should ask the user if he wants to save the changed profiles
 before exiting.

 The attached patch does this (except for two places, where I just 
 added a TODO instead of real code ;-) - probably solvable by 
 copypaste)

Ignore the except ... part - that's fixed in the v2 patch ;-)

 I don't really like that I had to remove the central handling of
 (F)inish in ui.py and add it to several places in aa.py. OTOH calling
 confirm_and_finish() from ui.py a) isn't that easy and b) would mix
 program logic into ui.py - I'm not sure if this is a good idea.
 
 Hmm, maybe we should also move the handling for Abo(r)t to aa.py and
 make ui.py totally dump to be consistent, even if this means to add
 some duplicated lines?
 
 Or add a small function to aa.py as wrapper for UI_PromptUser that
 - always adds (F)inish and Abo(r)t
 - handles those two
 - and returns any other answer to the calling function
 ?
 This function could then be used for all user prompts, with the
 exception of the save profile / view diff prompt.
 
 Opinions?


Here's an updated version of the patch that applies to current bzr trunk.

=== modified file 'utils/apparmor/aa.py'
--- utils/apparmor/aa.py2014-02-24 18:20:11 +
+++ utils/apparmor/aa.py2014-02-24 18:42:03 +
@@ -498,6 +498,10 @@
 
 ans = ''
 while 'CMD_USE_PROFILE' not in ans and 'CMD_CREATE_PROFILE' not in ans:
+if ans == 'CMD_FINISHED':
+save_profiles()
+return;
+
 ans, arg = aaui.UI_PromptUser(q)
 p = profile_hash[options[arg]]
 q['selected'] = options.index(options[arg])
@@ -990,6 +994,10 @@
 
 ans = aaui.UI_PromptUser(q)
 
+if ans == 'CMD_FINISHED':
+save_profiles()
+return;
+
 transitions[context] = ans
 
 if ans == 'CMD_ADDHAT':
@@ -1251,6 +1259,11 @@
 q['functions'] += build_x_functions(default, 
options, exec_toggle)
 ans = ''
 continue
+
+if ans == 'CMD_FINISHED':
+save_profiles()
+return;
+
 if ans == 'CMD_nx' or ans == 'CMD_nix':
 arg = exec_target
 ynans = 'n'
@@ -1550,6 +1563,11 @@
 done = False
 while not done:
 ans, selected = aaui.UI_PromptUser(q)
+
+if ans == 'CMD_FINISHED':
+save_profiles()
+return;
+
 # Ignore the log entry
 if ans == 'CMD_IGNORE_ENTRY':
 done = True
@@ -1794,6 +1812,10 @@
 
 ans, selected = aaui.UI_PromptUser(q)
 
+if ans == 'CMD_FINISHED':
+save_profiles()
+return;
+
 if ans == 'CMD_IGNORE_ENTRY':
 done = True
 break
@@ -1945,6 +1967,11 @@
 done = False
 while not done:
 ans, selected = aaui.UI_PromptUser(q)
+
+if ans == 'CMD_FINISHED':
+save_profiles()
+return;
+
 if ans == 'CMD_IGNORE_ENTRY':
 done = True
 break

=== modified file 'utils/apparmor/ui.py'
--- utils/apparmor/ui.py2014-02-13 18:01:03 +
+++ utils/apparmor/ui.py2014-02-24 18:32:40 +
@@ -288,10 +288,6 @@
 if cmd == 'CMD_ABORT':
 confirm_and_abort()
 cmd = 'XXXINVALIDXXX'
-elif cmd == 'CMD_FINISHED':
-if not params:
-confirm_and_finish()
-cmd = 'XXXINVALIDXXX'
 return (cmd, arg)
 
 def confirm_and_abort():
@@ -317,9 +313,6 @@
 })
 ypath, yarg = GetDataFromYast()
 
-def confirm_and_finish():
-sys.stdout.write(_('FINISHING...\n'))
-sys.exit(0)
 
 def Text_PromptUser(question):
 title = question['title']



Regards,

Christian Boltz
-- 
Wenn ich mir ein System installiere, welches dieses oder jenes oder
welches für mich tut (gegen mich?), dann kann ich gleich Windows
nehmen. Das fährt sich so gut, wie ein elektronischer Chauffeur das
eben kann: Nämlich bis die Elektronik ein Dixi-Klo mit 'ner Garage
verwechselt: Eckig und die Tür war offen.   [Ratti in suse-linux]


--

Re: [apparmor] [patch] new profile tools - handling of (F)inish

2014-02-24 Thread Kshitij Gupta 
On Feb 25, 2014 12:15 AM, Christian Boltz appar...@cboltz.de wrote:

 Hello,

 [patch v2, see below]

 Am Montag, 27. Januar 2014 schrieb Christian Boltz:
  currently, selecting (F)inish in the new profile tools basically means
  aborting without saving anything. However, we already have Abo(r)t
  for that ;-)
 
  (F)inish should ask the user if he wants to save the changed profiles
  before exiting.
 
  The attached patch does this (except for two places, where I just
  added a TODO instead of real code ;-) - probably solvable by
  copypaste)

 Ignore the except ... part - that's fixed in the v2 patch ;-)

  I don't really like that I had to remove the central handling of
  (F)inish in ui.py and add it to several places in aa.py. OTOH calling
  confirm_and_finish() from ui.py a) isn't that easy and b) would mix
  program logic into ui.py - I'm not sure if this is a good idea.
 
  Hmm, maybe we should also move the handling for Abo(r)t to aa.py and
  make ui.py totally dump to be consistent, even if this means to add
  some duplicated lines?
 
  Or add a small function to aa.py as wrapper for UI_PromptUser that
  - always adds (F)inish and Abo(r)t
  - handles those two
  - and returns any other answer to the calling function
  ?
  This function could then be used for all user prompts, with the
  exception of the save profile / view diff prompt.
 
  Opinions?

Sounds good. And as for save profile prompt, it needs some work as
discussed.

 Here's an updated version of the patch that applies to current bzr trunk.

 === modified file 'utils/apparmor/aa.py'
 --- utils/apparmor/aa.py2014-02-24 18:20:11 +
 +++ utils/apparmor/aa.py2014-02-24 18:42:03 +
 @@ -498,6 +498,10 @@

  ans = ''
  while 'CMD_USE_PROFILE' not in ans and 'CMD_CREATE_PROFILE' not in
ans:
 +if ans == 'CMD_FINISHED':
 +save_profiles()
 +return;
 +
  ans, arg = aaui.UI_PromptUser(q)
  p = profile_hash[options[arg]]
  q['selected'] = options.index(options[arg])
 @@ -990,6 +994,10 @@

  ans = aaui.UI_PromptUser(q)

 +if ans == 'CMD_FINISHED':
 +save_profiles()
 +return;
 +
  transitions[context] = ans

  if ans == 'CMD_ADDHAT':
 @@ -1251,6 +1259,11 @@
  q['functions'] +=
build_x_functions(default, options, exec_toggle)
  ans = ''
  continue
 +
 +if ans == 'CMD_FINISHED':
 +save_profiles()
 +return;
 +
  if ans == 'CMD_nx' or ans == 'CMD_nix':
  arg = exec_target
  ynans = 'n'
 @@ -1550,6 +1563,11 @@
  done = False
  while not done:
  ans, selected = aaui.UI_PromptUser(q)
 +
 +if ans == 'CMD_FINISHED':
 +save_profiles()
 +return;
 +
  # Ignore the log entry
  if ans == 'CMD_IGNORE_ENTRY':
  done = True
 @@ -1794,6 +1812,10 @@

  ans, selected = aaui.UI_PromptUser(q)

 +if ans == 'CMD_FINISHED':
 +save_profiles()
 +return;
 +
  if ans == 'CMD_IGNORE_ENTRY':
  done = True
  break
 @@ -1945,6 +1967,11 @@
  done = False
  while not done:
  ans, selected = aaui.UI_PromptUser(q)
 +
 +if ans == 'CMD_FINISHED':
 +save_profiles()
 +return;

I don't like the ; there after return. Besides that it looks good. Not
tested but I believe you did.

Acked-by: Kshitij Gupta, maybe remove the semicolon (Matter of taste).

 +
  if ans == 'CMD_IGNORE_ENTRY':
  done = True
  break

 === modified file 'utils/apparmor/ui.py'
 --- utils/apparmor/ui.py2014-02-13 18:01:03 +
 +++ utils/apparmor/ui.py2014-02-24 18:32:40 +
 @@ -288,10 +288,6 @@
  if cmd == 'CMD_ABORT':
  confirm_and_abort()
  cmd = 'XXXINVALIDXXX'
 -elif cmd == 'CMD_FINISHED':
 -if not params:
 -confirm_and_finish()
 -cmd = 'XXXINVALIDXXX'
  return (cmd, arg)

  def confirm_and_abort():
 @@ -317,9 +313,6 @@
  })
  ypath, yarg = GetDataFromYast()

 -def confirm_and_finish():
 -sys.stdout.write(_('FINISHING...\n'))
 -sys.exit(0)

  def

[apparmor] [patch] new profile tools - handling of (F)inish

2014-01-27 Thread Christian Boltz 
Hello,

currently, selecting (F)inish in the new profile tools basically means 
aborting without saving anything. However, we already have Abo(r)t for 
that ;-)

(F)inish should ask the user if he wants to save the changed profiles 
before exiting.

The attached patch does this (except for two places, where I just added 
a TODO instead of real code ;-) - probably solvable by copypaste)

I don't really like that I had to remove the central handling of 
(F)inish in ui.py and add it to several places in aa.py. OTOH calling 
confirm_and_finish() from ui.py a) isn't that easy and b) would mix 
program logic into ui.py - I'm not sure if this is a good idea.

Hmm, maybe we should also move the handling for Abo(r)t to aa.py and 
make ui.py totally dump to be consistent, even if this means to add some 
duplicated lines?

Or add a small function to aa.py as wrapper for UI_PromptUser that
- always adds (F)inish and Abo(r)t
- handles those two
- and returns any other answer to the calling function
?
This function could then be used for all user prompts, with the 
exception of the save profile / view diff prompt.

Opinions?


That all said:
Kshitij, this is your chance to write an evil review about a patch from 
me - don't waste it ;-)  (In the unlikely case that you like my patch, 
you can of course commit it ;-)


Regards,

Christian Boltz
-- 
Werbung lügt, Corporate Design sagt die Wahrheit. Naja,
alle _guten_ Komponenten der Wahrheit. :-)  [Ratti]
=== modified file 'Testing/runtests-py2.sh' (properties changed: -x to +x)
=== modified file 'Testing/runtests-py3.sh' (properties changed: -x to +x)
=== modified file 'apparmor/aa.py'
--- apparmor/aa.py	2013-12-19 21:42:58 +
+++ apparmor/aa.py	2014-01-27 21:52:05 +
@@ -492,6 +492,7 @@
 
 ans = ''
 while 'CMD_USE_PROFILE' not in ans and 'CMD_CREATE_PROFILE' not in ans:
+# TODO: handle FINISHED
 ans, arg = UI_PromptUser(q)
 p = profile_hash[options[arg]]
 q['selected'] = options.index(options[arg])
@@ -975,6 +979,8 @@
 
 ans = UI_PromptUser(q)
 
+# TODO: handle FINISHED
+
 transitions[context] = ans
 
 if ans == 'CMD_ADDHAT':
@@ -1236,6 +1242,11 @@
 q['functions'] += build_x_functions(default, options, exec_toggle)
 ans = ''
 continue
+
+if ans == 'CMD_FINISHED':
+save_profiles()
+return;
+
 if ans == 'CMD_nx' or ans == 'CMD_nix':
 arg = exec_target
 ynans = 'n'
@@ -1535,6 +1546,11 @@
 done = False
 while not done:
 ans, selected = UI_PromptUser(q)
+
+if ans == 'CMD_FINISHED':
+save_profiles()
+return;
+
 # Ignore the log entry
 if ans == 'CMD_IGNORE_ENTRY':
 done = True
@@ -1779,6 +1795,10 @@
 
 ans, selected = UI_PromptUser(q)
 
+if ans == 'CMD_FINISHED':
+save_profiles()
+return;
+
 if ans == 'CMD_IGNORE_ENTRY':
 done = True
 break
@@ -1930,6 +1950,11 @@
 done = False
 while not done:
 ans, selected = UI_PromptUser(q)
+
+if ans == 'CMD_FINISHED':
+save_profiles()
+return;
+
 if ans == 'CMD_IGNORE_ENTRY':
 done = True
 break
@@ -2224,7 +2249,7 @@
 pass
 
 finishing = False
-# Check for finished
+# TODO: Check for finished
 save_profiles()
 
 ##if not repo_cfg['repository'].get('upload', False) or repo['repository']['upload'] == 'later':

=== modified file 'apparmor/ui.py'
--- apparmor/ui.py	2013-12-29 09:42:30 +
+++ apparmor/ui.py	2014-01-27 22:00:57 +
@@ -288,10 +288,10 @@
 if cmd == 'CMD_ABORT':
 confirm_and_abort()
 cmd = 'XXXINVALIDXXX'
-elif cmd == 'CMD_FINISHED':
-if not params:
-confirm_and_finish()
-cmd = 'XXXINVALIDXXX'
+#elif cmd == 'CMD_FINISHED':
+#if not params:
+#confirm_and_finish()
+#cmd = 'XXXINVALIDXXX'
 return (cmd, arg)
 
 def confirm_and_abort():
@@ -319,9 +319,9 @@
 })
 ypath, yarg = GetDataFromYast()
 
-def confirm_and_finish():
-sys.stdout.write(_('FINISHING...\n'))
-sys.exit(0)
+#def confirm_and_finish():
+#