Some code review

1 files changed, 240 insertions, 238 deletions

diff --git a/src/slic3r/GUI/Field.cpp b/src/slic3r/GUI/Field.cpp
index 8efb4ff30..518652e90 100644
--- a/src/slic3r/GUI/Field.cpp
+++ b/src/slic3r/GUI/Field.cpp
@@ -11,274 +11,275 @@
 
 namespace Slic3r { namespace GUI {
 
-	wxString double_to_string(double const value)
-	{
-		if (value - int(value) == 0)
-			return wxString::Format(_T("%i"), int(value));
-		else {
-			int precision = 4;
-			for (size_t p = 1; p < 4; p++)
-			{
-				double cur_val = pow(10, p)*value;
-				if (cur_val - int(cur_val) == 0) {
-					precision = p;
-					break;
-				}
+wxString double_to_string(double const value)
+{
+	if (value - int(value) == 0)
+		return wxString::Format(_T("%i"), int(value));
+	else {
+		int precision = 4;
+		for (size_t p = 1; p < 4; p++)
+		{
+			double cur_val = pow(10, p)*value;
+			if (cur_val - int(cur_val) == 0) {
+				precision = p;
+				break;
 			}
-			return wxNumberFormatter::ToString(value, precision, wxNumberFormatter::Style_None);
 		}
+		return wxNumberFormatter::ToString(value, precision, wxNumberFormatter::Style_None);
 	}
+}
 
-	void Field::PostInitialize()
-    {
-		auto color = wxSystemSettings::GetColour(wxSYS_COLOUR_WINDOW);
-		m_Undo_btn			= new MyButton(m_parent, wxID_ANY, "", wxDefaultPosition,wxDefaultSize, wxBU_EXACTFIT | wxNO_BORDER);
-		m_Undo_to_sys_btn	= new MyButton(m_parent, wxID_ANY, "", wxDefaultPosition,wxDefaultSize, wxBU_EXACTFIT | wxNO_BORDER);
-		if (wxMSW) {
-			m_Undo_btn->SetBackgroundColour(color);
-			m_Undo_to_sys_btn->SetBackgroundColour(color);
-		}
-		m_Undo_btn->Bind(wxEVT_BUTTON, ([this](wxCommandEvent){ on_back_to_initial_value(); }));
-		m_Undo_to_sys_btn->Bind(wxEVT_BUTTON, ([this](wxCommandEvent){ on_back_to_sys_value(); }));
-
-		//set default bitmap
-		wxBitmap bmp;
-		bmp.LoadFile(from_u8(var("bullet_white.png")), wxBITMAP_TYPE_PNG);
-		set_undo_bitmap(&bmp);
-		set_undo_to_sys_bitmap(&bmp);
-
-		switch (m_opt.type)
-		{
-		case coPercents:
-		case coFloats:
-		case coStrings:	
-		case coBools:		
-		case coInts: {
-			auto tag_pos = m_opt_id.find("#");
-			if (tag_pos != std::string::npos)
-				m_opt_idx = stoi(m_opt_id.substr(tag_pos + 1, m_opt_id.size()));
-			break;
-		}
-		default:
-			break;
-		}
-
-		BUILD();
+void Field::PostInitialize()
+{
+	auto color = wxSystemSettings::GetColour(wxSYS_COLOUR_WINDOW);
+	m_Undo_btn			= new MyButton(m_parent, wxID_ANY, "", wxDefaultPosition,wxDefaultSize, wxBU_EXACTFIT | wxNO_BORDER);
+	m_Undo_to_sys_btn	= new MyButton(m_parent, wxID_ANY, "", wxDefaultPosition,wxDefaultSize, wxBU_EXACTFIT | wxNO_BORDER);
+	if (wxMSW) {
+		m_Undo_btn->SetBackgroundColour(color);
+		m_Undo_to_sys_btn->SetBackgroundColour(color);
 	}
+	m_Undo_btn->Bind(wxEVT_BUTTON, ([this](wxCommandEvent) { on_back_to_initial_value(); }));
+	m_Undo_to_sys_btn->Bind(wxEVT_BUTTON, ([this](wxCommandEvent) { on_back_to_sys_value(); }));
 
-	void Field::on_kill_focus(wxEvent& event)
-    {
-        // Without this, there will be nasty focus bugs on Windows.
-        // Also, docs for wxEvent::Skip() say "In general, it is recommended to skip all 
-        // non-command events to allow the default handling to take place."
-		event.Skip();
-		// call the registered function if it is available
-        if (m_on_kill_focus!=nullptr) 
-            m_on_kill_focus();
-    }
-    void Field::on_change_field()
+	//set default bitmap
+	wxBitmap bmp;
+	bmp.LoadFile(from_u8(var("bullet_white.png")), wxBITMAP_TYPE_PNG);
+	set_undo_bitmap(&bmp);
+	set_undo_to_sys_bitmap(&bmp);
+
+	switch (m_opt.type)
 	{
-//        std::cerr << "calling Field::_on_change \n";
-        if (m_on_change != nullptr  && !m_disable_change_event)
-            m_on_change(m_opt_id, get_value());
-    }
-
-	void Field::on_back_to_initial_value()
-    {
-		if (m_back_to_initial_value != nullptr && m_is_modified_value)
-			m_back_to_initial_value(m_opt_id);
+	case coPercents:
+	case coFloats:
+	case coStrings:	
+	case coBools:		
+	case coInts: {
+		auto tag_pos = m_opt_id.find("#");
+		if (tag_pos != std::string::npos)
+			m_opt_idx = stoi(m_opt_id.substr(tag_pos + 1, m_opt_id.size()));
+		break;
 	}
-
-	void Field::on_back_to_sys_value()
-    {
-		if (m_back_to_sys_value != nullptr && m_is_nonsys_value)
-			m_back_to_sys_value(m_opt_id);
+	default:
+		break;
 	}
 
-	wxString Field::get_tooltip_text(const wxString& default_string)
-	{
-		wxString tooltip_text("");
-		wxString tooltip = _(m_opt.tooltip);
-		if (tooltip.length() > 0)
-            tooltip_text = tooltip + "\n" + _(L("default value")) + "\t: " +
-            (boost::iends_with(m_opt_id, "_gcode") ? "\n" : "") + default_string +
-            (boost::iends_with(m_opt_id, "_gcode") ? "" : "\n") + 
-            _(L("parameter name")) + "\t: " + m_opt_id;
-
-		return tooltip_text;
-	}
+	BUILD();
+}
 
-	bool Field::is_matched(const std::string& string, const std::string& pattern)
-	{
-		std::regex regex_pattern(pattern, std::regex_constants::icase); // use ::icase to make the matching case insensitive like /i in perl
-		return std::regex_match(string, regex_pattern);
-	}
+void Field::on_kill_focus(wxEvent& event)
+{
+    // Without this, there will be nasty focus bugs on Windows.
+    // Also, docs for wxEvent::Skip() say "In general, it is recommended to skip all 
+    // non-command events to allow the default handling to take place."
+	event.Skip();
+	// call the registered function if it is available
+    if (m_on_kill_focus!=nullptr) 
+        m_on_kill_focus();
+}
 
-	void Field::get_value_by_opt_type(wxString& str)
-	{
-		switch (m_opt.type){
-		case coInt:
-			m_value = wxAtoi(str);
-			break;
-		case coPercent:
-		case coPercents:
-		case coFloats:
-		case coFloat:{
-			if (m_opt.type == coPercent && str.Last() == '%') 
-				str.RemoveLast();
-			else if (str.Last() == '%')	{
-				wxString label = m_Label->GetLabel();
-				if		(label.Last() == '\n')	label.RemoveLast();
-				while	(label.Last() == ' ')	label.RemoveLast();
-				if		(label.Last() == ':')	label.RemoveLast();
-				show_error(m_parent, wxString::Format(_(L("%s doesn't support percentage")), label));
-				set_value(double_to_string(m_opt.min), true);
-				m_value = double(m_opt.min);
-				break;
-			}
-			double val;
-			if(!str.ToCDouble(&val))
-			{
-				show_error(m_parent, _(L("Input value contains incorrect symbol(s).\nUse, please, only digits")));
-				set_value(double_to_string(val), true);
-			}
-			if (m_opt.min > val || val > m_opt.max)
-			{
-				show_error(m_parent, _(L("Input value is out of range")));
-				if (m_opt.min > val) val = m_opt.min;
-				if (val > m_opt.max) val = m_opt.max;
-				set_value(double_to_string(val), true);
-			}
-			m_value = val;
-			break; }
-		case coString:
-		case coStrings:
-		case coFloatOrPercent:
-			m_value = str.ToStdString();
-			break;
-		default:
-			break;
-		}
-	}
+void Field::on_change_field()
+{
+      std::cerr << "calling Field::_on_change \n";
+    if (m_on_change != nullptr  && !m_disable_change_event)
+        m_on_change(m_opt_id, get_value());
+}
+
+void Field::on_back_to_initial_value()
+{
+	if (m_back_to_initial_value != nullptr && m_is_modified_value)
+		m_back_to_initial_value(m_opt_id);
+}
+
+void Field::on_back_to_sys_value()
+{
+	if (m_back_to_sys_value != nullptr && m_is_nonsys_value)
+		m_back_to_sys_value(m_opt_id);
+}
 
-	void TextCtrl::BUILD() {
-        auto size = wxSize(wxDefaultSize);
-        if (m_opt.height >= 0) size.SetHeight(m_opt.height);
-        if (m_opt.width >= 0) size.SetWidth(m_opt.width);
+wxString Field::get_tooltip_text(const wxString& default_string)
+{
+	wxString tooltip_text("");
+	wxString tooltip = _(m_opt.tooltip);
+	if (tooltip.length() > 0)
+        tooltip_text = tooltip + "\n" + _(L("default value")) + "\t: " +
+        (boost::iends_with(m_opt_id, "_gcode") ? "\n" : "") + default_string +
+        (boost::iends_with(m_opt_id, "_gcode") ? "" : "\n") + 
+        _(L("parameter name")) + "\t: " + m_opt_id;
+
+	return tooltip_text;
+}
 
-		wxString text_value = wxString(""); 
+bool Field::is_matched(const std::string& string, const std::string& pattern)
+{
+	std::regex regex_pattern(pattern, std::regex_constants::icase); // use ::icase to make the matching case insensitive like /i in perl
+	return std::regex_match(string, regex_pattern);
+}
 
-		switch (m_opt.type) {
-		case coFloatOrPercent:
-		{
-			text_value = double_to_string(m_opt.default_value->getFloat());
- 			if (static_cast<const ConfigOptionFloatOrPercent*>(m_opt.default_value)->percent)
- 				text_value += "%";
+void Field::get_value_by_opt_type(wxString& str)
+{
+	switch (m_opt.type) {
+	case coInt:
+		m_value = wxAtoi(str);
+		break;
+	case coPercent:
+	case coPercents:
+	case coFloats:
+	case coFloat:{
+		if (m_opt.type == coPercent && str.Last() == '%') 
+			str.RemoveLast();
+		else if (str.Last() == '%')	{
+			wxString label = m_Label->GetLabel();
+			if		(label.Last() == '\n')	label.RemoveLast();
+			while	(label.Last() == ' ')	label.RemoveLast();
+			if		(label.Last() == ':')	label.RemoveLast();
+			show_error(m_parent, wxString::Format(_(L("%s doesn't support percentage")), label));
+			set_value(double_to_string(m_opt.min), true);
+			m_value = double(m_opt.min);
 			break;
 		}
-		case coPercent:
-		{
-			text_value = wxString::Format(_T("%i"), int(m_opt.default_value->getFloat()));
-			text_value += "%";
-			break;
-		}	
-		case coPercents:
-		case coFloats:
-		case coFloat:
+		double val;
+		if(!str.ToCDouble(&val))
 		{
-			double val = m_opt.type == coFloats ?
-				static_cast<const ConfigOptionFloats*>(m_opt.default_value)->get_at(m_opt_idx) :
-				m_opt.type == coFloat ? 
-					m_opt.default_value->getFloat() :
-					static_cast<const ConfigOptionPercents*>(m_opt.default_value)->get_at(m_opt_idx);
-			text_value = double_to_string(val);
-			break;
+			show_error(m_parent, _(L("Input value contains incorrect symbol(s).\nUse, please, only digits")));
+			set_value(double_to_string(val), true);
 		}
-		case coString:			
-			text_value = static_cast<const ConfigOptionString*>(m_opt.default_value)->value;
-			break;
-		case coStrings:
+		if (m_opt.min > val || val > m_opt.max)
 		{
-			const ConfigOptionStrings *vec = static_cast<const ConfigOptionStrings*>(m_opt.default_value);
-			if (vec == nullptr || vec->empty()) break; //for the case of empty default value
-			text_value = vec->get_at(m_opt_idx);
-			break;
-		}
-		default:
-			break; 
+			show_error(m_parent, _(L("Input value is out of range")));
+			if (m_opt.min > val) val = m_opt.min;
+			if (val > m_opt.max) val = m_opt.max;
+			set_value(double_to_string(val), true);
 		}
+		m_value = val;
+		break; }
+	case coString:
+	case coStrings:
+	case coFloatOrPercent:
+		m_value = str.ToStdString();
+		break;
+	default:
+		break;
+	}
+}
 
-		auto temp = new wxTextCtrl(m_parent, wxID_ANY, text_value, wxDefaultPosition, size, (m_opt.multiline ? wxTE_MULTILINE : 0));
+void TextCtrl::BUILD() {
+    auto size = wxSize(wxDefaultSize);
+    if (m_opt.height >= 0) size.SetHeight(m_opt.height);
+    if (m_opt.width >= 0) size.SetWidth(m_opt.width);
 
-		temp->SetToolTip(get_tooltip_text(text_value));
-        
-		temp->Bind(wxEVT_LEFT_DOWN, ([temp](wxEvent& event)
-		{
-			//! to allow the default handling
-			event.Skip();
-			//! eliminating the g-code pop up text description
-			bool flag = false;
+	wxString text_value = wxString(""); 
+
+	switch (m_opt.type) {
+	case coFloatOrPercent:
+	{
+		text_value = double_to_string(m_opt.default_value->getFloat());
+		if (static_cast<const ConfigOptionFloatOrPercent*>(m_opt.default_value)->percent)
+			text_value += "%";
+		break;
+	}
+	case coPercent:
+	{
+		text_value = wxString::Format(_T("%i"), int(m_opt.default_value->getFloat()));
+		text_value += "%";
+		break;
+	}	
+	case coPercents:
+	case coFloats:
+	case coFloat:
+	{
+		double val = m_opt.type == coFloats ?
+			static_cast<const ConfigOptionFloats*>(m_opt.default_value)->get_at(m_opt_idx) :
+			m_opt.type == coFloat ? 
+				m_opt.default_value->getFloat() :
+				static_cast<const ConfigOptionPercents*>(m_opt.default_value)->get_at(m_opt_idx);
+		text_value = double_to_string(val);
+		break;
+	}
+	case coString:			
+		text_value = static_cast<const ConfigOptionString*>(m_opt.default_value)->value;
+		break;
+	case coStrings:
+	{
+		const ConfigOptionStrings *vec = static_cast<const ConfigOptionStrings*>(m_opt.default_value);
+		if (vec == nullptr || vec->empty()) break; //for the case of empty default value
+		text_value = vec->get_at(m_opt_idx);
+		break;
+	}
+	default:
+		break; 
+	}
+
+	auto temp = new wxTextCtrl(m_parent, wxID_ANY, text_value, wxDefaultPosition, size, (m_opt.multiline ? wxTE_MULTILINE : 0));
+
+	temp->SetToolTip(get_tooltip_text(text_value));
+    
+	temp->Bind(wxEVT_LEFT_DOWN, ([temp](wxEvent& event)
+	{
+		//! to allow the default handling
+		event.Skip();
+		//! eliminating the g-code pop up text description
+		bool flag = false;
 #ifdef __WXGTK__
-			// I have no idea why, but on GTK flag works in other way
-			flag = true;
+		// I have no idea why, but on GTK flag works in other way
+		flag = true;
 #endif // __WXGTK__
-			temp->GetToolTip()->Enable(flag);
-		}), temp->GetId());
+		temp->GetToolTip()->Enable(flag);
+	}), temp->GetId());
 
 #if !defined(__WXGTK__)
-		temp->Bind(wxEVT_KILL_FOCUS, ([this, temp](wxEvent& e)
-		{
-			e.Skip();//	on_kill_focus(e);
-			temp->GetToolTip()->Enable(true);
-		}), temp->GetId());
+	temp->Bind(wxEVT_KILL_FOCUS, ([this, temp](wxEvent& e)
+	{
+		e.Skip();//	on_kill_focus(e);
+		temp->GetToolTip()->Enable(true);
+	}), temp->GetId());
 #endif // __WXGTK__
 
-		temp->Bind(wxEVT_TEXT, ([this](wxCommandEvent& evt)
-		{
+	temp->Bind(wxEVT_TEXT, ([this](wxCommandEvent& evt)
+	{
 #ifdef __WXGTK__
-			if (bChangedValueEvent)
+        if (bChangedValueEvent)
 #endif //__WXGTK__
-			on_change_field();
-		}), temp->GetId());
+		on_change_field();
+	}), temp->GetId());
 
 #ifdef __WXGTK__
-        // to correct value updating on GTK we should:
-        // call on_change_field() on wxEVT_KEY_UP instead of wxEVT_TEXT
-        // and prevent value updating on wxEVT_KEY_DOWN
-        temp->Bind(wxEVT_KEY_DOWN, &TextCtrl::change_field_value, this);
-        temp->Bind(wxEVT_KEY_UP, &TextCtrl::change_field_value, this);
+    // to correct value updating on GTK we should:
+    // call on_change_field() on wxEVT_KEY_UP instead of wxEVT_TEXT
+    // and prevent value updating on wxEVT_KEY_DOWN
+    temp->Bind(wxEVT_KEY_DOWN, &TextCtrl::change_field_value, this);
+    temp->Bind(wxEVT_KEY_UP, &TextCtrl::change_field_value, this);
 #endif //__WXGTK__
 
-		// select all text using Ctrl+A
-		temp->Bind(wxEVT_CHAR, ([temp](wxKeyEvent& event)
-		{
-			if (wxGetKeyState(wxKeyCode('A')) && wxGetKeyState(WXK_CONTROL))
-				temp->SetSelection(-1, -1); //select all
-			event.Skip();
-		}));
+	// select all text using Ctrl+A
+	temp->Bind(wxEVT_CHAR, ([temp](wxKeyEvent& event)
+	{
+		if (wxGetKeyState(wxKeyCode('A')) && wxGetKeyState(WXK_CONTROL))
+			temp->SetSelection(-1, -1); //select all
+		event.Skip();
+	}));
 
-        // recast as a wxWindow to fit the calling convention
-        window = dynamic_cast<wxWindow*>(temp);
-    }	
+    // recast as a wxWindow to fit the calling convention
+    window = dynamic_cast<wxWindow*>(temp);
+}	
 
-	boost::any& TextCtrl::get_value()
-	{
-		wxString ret_str = static_cast<wxTextCtrl*>(window)->GetValue();
-		get_value_by_opt_type(ret_str);
+boost::any& TextCtrl::get_value()
+{
+	wxString ret_str = static_cast<wxTextCtrl*>(window)->GetValue();
+	get_value_by_opt_type(ret_str);
 
-		return m_value;
-	}
+	return m_value;
+}
 
-	void TextCtrl::enable() { dynamic_cast<wxTextCtrl*>(window)->Enable(); dynamic_cast<wxTextCtrl*>(window)->SetEditable(true); }
-    void TextCtrl::disable() { dynamic_cast<wxTextCtrl*>(window)->Disable(); dynamic_cast<wxTextCtrl*>(window)->SetEditable(false); }
+void TextCtrl::enable() { dynamic_cast<wxTextCtrl*>(window)->Enable(); dynamic_cast<wxTextCtrl*>(window)->SetEditable(true); }
+void TextCtrl::disable() { dynamic_cast<wxTextCtrl*>(window)->Disable(); dynamic_cast<wxTextCtrl*>(window)->SetEditable(false); }
 
 #ifdef __WXGTK__
-    void TextCtrl::change_field_value(wxEvent& event)
-    {
-    	if (bChangedValueEvent = event.GetEventType()==wxEVT_KEY_UP)
-    		on_change_field();
-        event.Skip();
-    };
+void TextCtrl::change_field_value(wxEvent& event)
+{
+	if (bChangedValueEvent = event.GetEventType()==wxEVT_KEY_UP)
+		on_change_field();
+    event.Skip();
+};
 #endif //__WXGTK__
 
 void CheckBox::BUILD() {
@@ -388,10 +389,10 @@ void Choice::BUILD() {
 	// recast as a wxWindow to fit the calling convention
 	window = dynamic_cast<wxWindow*>(temp);
 
-	if (m_opt.enum_labels.empty() && m_opt.enum_values.empty()){
+	if (m_opt.enum_labels.empty() && m_opt.enum_values.empty()) {
 	}
 	else{
-		for (auto el : m_opt.enum_labels.empty() ? m_opt.enum_values : m_opt.enum_labels){
+		for (auto el : m_opt.enum_labels.empty() ? m_opt.enum_values : m_opt.enum_labels) {
 			const wxString& str = _(el);//m_opt_id == "support" ? _(el) : el;
 			temp->Append(str);
 		}
@@ -406,7 +407,7 @@ void Choice::BUILD() {
 void Choice::set_selection()
 {
 	wxString text_value = wxString("");
-	switch (m_opt.type){
+	switch (m_opt.type) {
 	case coFloat:
 	case coPercent:	{
 		double val = m_opt.default_value->getFloat();
@@ -485,12 +486,12 @@ void Choice::set_value(const boost::any& value, bool change_event)
 {
 	m_disable_change_event = !change_event;
 
-	switch (m_opt.type){
+	switch (m_opt.type) {
 	case coInt:
 	case coFloat:
 	case coPercent:
 	case coString:
-	case coStrings:{
+	case coStrings: {
 		wxString text_value;
 		if (m_opt.type == coInt) 
 			text_value = wxString::Format(_T("%i"), int(boost::any_cast<int>(value)));
@@ -508,11 +509,11 @@ void Choice::set_value(const boost::any& value, bool change_event)
 			dynamic_cast<wxComboBox*>(window)->SetSelection(idx);
 		break;
 	}
-	case coEnum:{
+	case coEnum: {
 		int val = boost::any_cast<int>(value);
 		if (m_opt_id.compare("external_fill_pattern") == 0)
 		{
-			if (!m_opt.enum_values.empty()){
+			if (!m_opt.enum_values.empty()) {
 				std::string key;
 				t_config_enum_values map_names = ConfigOptionEnum<InfillPattern>::get_enum_values();				
 				for (auto it : map_names) {
@@ -583,7 +584,7 @@ boost::any& Choice::get_value()
 		int ret_enum = static_cast<wxComboBox*>(window)->GetSelection(); 
 		if (m_opt_id.compare("external_fill_pattern") == 0)
 		{
-			if (!m_opt.enum_values.empty()){
+			if (!m_opt.enum_values.empty()) {
 				std::string key = m_opt.enum_values[ret_enum];
 				t_config_enum_values map_names = ConfigOptionEnum<InfillPattern>::get_enum_values();
 				int value = map_names.at(key);
@@ -631,7 +632,8 @@ void ColourPicker::BUILD()
 	temp->SetToolTip(get_tooltip_text(clr_str));
 }
 
-boost::any& ColourPicker::get_value(){
+boost::any& ColourPicker::get_value()
+{
 // 	boost::any m_value;
 
 	auto colour = static_cast<wxColourPickerCtrl*>(window)->GetColour();
@@ -750,7 +752,7 @@ void SliderCtrl::BUILD()
 	temp->Add(m_textctrl, 0, wxALIGN_CENTER_VERTICAL, 0);
 
 	m_slider->Bind(wxEVT_SLIDER, ([this](wxCommandEvent e) {
-		if (!m_disable_change_event){
+		if (!m_disable_change_event) {
 			int val = boost::any_cast<int>(get_value());
 			m_textctrl->SetLabel(wxString::Format("%d", val));
 			on_change_field();
@@ -759,7 +761,7 @@ void SliderCtrl::BUILD()
 
 	m_textctrl->Bind(wxEVT_TEXT, ([this](wxCommandEvent e) {
 		std::string value = e.GetString().utf8_str().data();
-		if (is_matched(value, "^-?\\d+(\\.\\d*)?$")){
+		if (is_matched(value, "^-?\\d+(\\.\\d*)?$")) {
 			m_disable_change_event = true;
 			m_slider->SetValue(stoi(value)*m_scale);
 			m_disable_change_event = false;