[yast-devel] Re: [yast-commit] <web-client> network : correctly handle netmask/prefixlen (bnc#539889)
Michal Zugec napsal(a):
ref: refs/heads/network commit 409cd7667168c6d03628ef0cc3b78f1fb280a54f Author: Michal Zugec <mzugec@suse.cz> Date: Mon Sep 28 21:56:38 2009 +0200
correctly handle netmask/prefixlen (bnc#539889) --- .../network/app/controllers/network_controller.rb | 16 +++++----------- 1 files changed, 5 insertions(+), 11 deletions(-)
diff --git a/plugins/network/app/controllers/network_controller.rb b/plugins/network/app/controllers/network_controller.rb index 65c8f46..8c82060 100644 --- a/plugins/network/app/controllers/network_controller.rb +++ b/plugins/network/app/controllers/network_controller.rb @@ -50,26 +50,20 @@ class NetworkController < ApplicationController ipaddr = "-/-" end @ip, @netmask = ipaddr.split "/" - + # when detect PREFIXLEN with leading "/" + @netmask = "/"+@netmask if ifc.bootproto == "static" && @netmask.to_i >= 0 && @netmask.to_i <= 32 ^^^ I think that this complex condition should be in separated if statement. This version is not much readable. I suggest use constant for netmask and test range. Somethink like: NETMASK_RANGE = 0..32 if ifc.bootproto == "static" && NETMASK_RANGE.include?(netmask.to_i) @netmask = "/"+@netmask end
+ @name = hn.name @domain = hn.domain @nameservers = dns.nameservers @searchdomains = dns.searches @default_route = rt.via - #FIXME: this is ugly and keys are duplicated, but otherwise seems it doesn't work - @conf_modes = [["",""], ["static","static"], ["dhcp", "dhcp"]] - # if unknown item, just add it into list - found = false - @conf_modes.each {|a| found=true if a[0] == @conf_mode} - @conf_modes << [@conf_mode, @conf_mode] if !found + @conf_modes = {""=>"", _("static")=>"static", _("dhcp")=>"dhcp"} + @conf_modes[@conf_mode] ||=@conf_mode
Hi, you mix two sollution which I suggest to you and this doesn't work For example you have static...translated to cz is staticky and on second line you test if exist @conf_modes["static"] but you have @conf_mode["staticky"] so this is not produce valid output. As I suggest revert this hash (so conf_mode is keys and localized strings is values) and in options_for use @conf_modes.invert. When I come to work we should look at it together.
end
- # GET /users/1/edit - def edit - end -
# PUT /users/1 # PUT /users/1.xml
-- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org For additional commands, e-mail: yast-devel+help@opensuse.org
josef reidinger napsal(a):
Michal Zugec napsal(a):
ref: refs/heads/network commit 409cd7667168c6d03628ef0cc3b78f1fb280a54f Author: Michal Zugec <mzugec@suse.cz> Date: Mon Sep 28 21:56:38 2009 +0200
correctly handle netmask/prefixlen (bnc#539889) --- .../network/app/controllers/network_controller.rb | 16 +++++----------- 1 files changed, 5 insertions(+), 11 deletions(-)
diff --git a/plugins/network/app/controllers/network_controller.rb b/plugins/network/app/controllers/network_controller.rb index 65c8f46..8c82060 100644 --- a/plugins/network/app/controllers/network_controller.rb +++ b/plugins/network/app/controllers/network_controller.rb @@ -50,26 +50,20 @@ class NetworkController < ApplicationController ipaddr = "-/-" end @ip, @netmask = ipaddr.split "/" - + # when detect PREFIXLEN with leading "/" + @netmask = "/"+@netmask if ifc.bootproto == "static" && @netmask.to_i >= 0 && @netmask.to_i <= 32 ^^^ I think that this complex condition should be in separated if statement. This version is not much readable. I suggest use constant for netmask and test range. Somethink like: NETMASK_RANGE = 0..32 if ifc.bootproto == "static" && NETMASK_RANGE.include?(netmask.to_i) Another solution is NETMASK_RANGE === netmask.to_i I don't know which is better readable that we test if number is in interval. @netmask = "/"+@netmask end
+ @name = hn.name @domain = hn.domain @nameservers = dns.nameservers @searchdomains = dns.searches @default_route = rt.via - #FIXME: this is ugly and keys are duplicated, but otherwise seems it doesn't work - @conf_modes = [["",""], ["static","static"], ["dhcp", "dhcp"]] - # if unknown item, just add it into list - found = false - @conf_modes.each {|a| found=true if a[0] == @conf_mode} - @conf_modes << [@conf_mode, @conf_mode] if !found + @conf_modes = {""=>"", _("static")=>"static", _("dhcp")=>"dhcp"} + @conf_modes[@conf_mode] ||=@conf_mode
Hi, you mix two sollution which I suggest to you and this doesn't work For example you have static...translated to cz is staticky and on second line you test if exist @conf_modes["static"] but you have @conf_mode["staticky"] so this is not produce valid output. As I suggest revert this hash (so conf_mode is keys and localized strings is values) and in options_for use @conf_modes.invert. When I come to work we should look at it together.
end
- # GET /users/1/edit - def edit - end -
# PUT /users/1 # PUT /users/1.xml
-- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org For additional commands, e-mail: yast-devel+help@opensuse.org
participants (1)
-
josef reidinger