From 2be5415d4a5d024c223d340381ef74e3697fc7ea Mon Sep 17 00:00:00 2001 From: Siri Hansen Date: Wed, 29 Feb 2012 16:30:13 +0100 Subject: [reltool] Display warnings continously in GUI OTP-9967 All active warnings are now displayed in a specific warning list at the bottom of the sys windows. Warnings do no longer cause popup dialogs during configuration. The reason for this is to avoid the same warning to pop up many times. This would happen since each configuration change now causes a fresh reading of the file system - and thus each warning would be detected each time the configuration was changed. reltool_manual_gui_SUITE is updated to test the new functionality. --- lib/reltool/src/reltool_sys_win.erl | 207 +++++++++++++++++++++----- lib/reltool/src/reltool_utils.erl | 12 +- lib/reltool/test/reltool_manual_gui_SUITE.erl | 48 ++++-- 3 files changed, 215 insertions(+), 52 deletions(-) (limited to 'lib/reltool') diff --git a/lib/reltool/src/reltool_sys_win.erl b/lib/reltool/src/reltool_sys_win.erl index c75b302770..29a01b63d8 100644 --- a/lib/reltool/src/reltool_sys_win.erl +++ b/lib/reltool/src/reltool_sys_win.erl @@ -56,7 +56,9 @@ derived, fgraph_wins, app_box, - mod_box + mod_box, + warning_list, + warning_wins }). -define(WIN_WIDTH, 800). @@ -88,6 +90,10 @@ -define(blacklist, "Excluded"). -define(derived, "Derived"). +-define(WARNING_COL, 0). +-define(DEFAULT_WARNING_TIP, "Warnings are listed in this window"). +-define(WARNING_POPUP_SIZE, {400,150}). + -define(safe_config,{sys,[{incl_cond,exclude}, {app,kernel,[{incl_cond,include}]}, {app,stdlib,[{incl_cond,include}]}, @@ -150,7 +156,8 @@ do_init([{safe_config, Safe}, {parent, Parent} | Options]) -> target_dir = filename:absname("reltool_target_dir"), app_wins = [], sys = Sys, - fgraph_wins = []}, + fgraph_wins = [], + warning_wins = []}, S2 = create_window(S), S5 = wx:batch(fun() -> Title = atom_to_list(?APPLICATION), @@ -235,10 +242,18 @@ loop(S) -> lists:keydelete(ObjRef, #fgraph_win.frame, FWs), ?MODULE:loop(S#state{fgraph_wins = FWs2}); false -> - error_logger:format("~p~p got unexpected " - "message:\n\t~p\n", - [?MODULE, self(), Msg]), - ?MODULE:loop(S) + WWs = S#state.warning_wins, + case lists:member(ObjRef, WWs) of + true -> + wxFrame:destroy(ObjRef), + WWs2 = lists:delete(ObjRef, WWs), + ?MODULE:loop(S#state{warning_wins = WWs2}); + false -> + error_logger:format("~p~p got unexpected " + "message:\n\t~p\n", + [?MODULE, self(), Msg]), + ?MODULE:loop(S) + end end end; #wx{id = ?CLOSE_ITEM, @@ -292,8 +307,8 @@ handle_child_exit({'EXIT', Pid, _Reason} = Exit, FWs, AWs) -> msg_warning(Exit, application_window), {FWs, lists:keydelete(Pid, #app_win.pid, AWs)}; false -> - msg_warning(Exit, unknown), - {FWs, AWs} + msg_warning(Exit, unknown), + {FWs, AWs} end end. @@ -335,8 +350,12 @@ create_window(S) -> fun create_main_release_page/1, fun create_config_page/1 ]), + + S4 = create_warning_list(S3), + Sizer = wxBoxSizer:new(?wxVERTICAL), wxSizer:add(Sizer, Book, [{flag, ?wxEXPAND}, {proportion, 1}]), + wxSizer:add(Sizer, S4#state.warning_list, [{flag, ?wxEXPAND}]), wxPanel:setSizer(Panel, Sizer), wxSizer:fit(Sizer, Frame), @@ -344,7 +363,7 @@ create_window(S) -> wxFrame:connect(Frame, close_window), wxFrame:show(Frame), - S3. + S4. create_menubar(Frame) -> MenuBar = wxMenuBar:new(), @@ -637,6 +656,48 @@ redraw_config_page(#state{sys = Sys, app_box = AppBox, mod_box = ModBox} = S) -> wxRadioBox:setSelection(ModBox, ModChoice), S. +create_warning_list(#state{panel = Panel} = S) -> + ListCtrl = wxListCtrl:new(Panel, + [{style, + ?wxLC_REPORT bor + ?wxLC_HRULES bor + ?wxVSCROLL}, + {size, {?WIN_WIDTH,80}}]), + reltool_utils:assign_image_list(ListCtrl), + wxListCtrl:insertColumn(ListCtrl, ?WARNING_COL, "Warnings", + [{format,?wxLIST_FORMAT_LEFT}]), + wxListCtrl:setToolTip(ListCtrl, ?DEFAULT_WARNING_TIP), + wxEvtHandler:connect(ListCtrl, size, + [{skip, true}, {userData, warnings}]), + wxEvtHandler:connect(ListCtrl, command_list_item_activated, + [{userData, warnings}]), + wxEvtHandler:connect(ListCtrl, motion, [{userData, warnings}]), + wxEvtHandler:connect(ListCtrl, enter_window), + S#state{warning_list=ListCtrl}. + +redraw_warnings(S) -> + {ok,Warnings} = reltool_server:get_status(S#state.server_pid), + redraw_warnings(S#state.warning_list,Warnings), + length(Warnings). + +redraw_warnings(ListCtrl, []) -> + wxListCtrl:deleteAllItems(ListCtrl), + ok; +redraw_warnings(ListCtrl, Warnings) -> + wxListCtrl:deleteAllItems(ListCtrl), + Show = fun(Warning, Row) -> + wxListCtrl:insertItem(ListCtrl, Row, ""), + wxListCtrl:setItem(ListCtrl, + Row, + ?WARNING_COL, + Warning, + [{imageId, ?WARN_IMAGE}]), + Row + 1 + end, + wx:foldl(Show, 0, Warnings), + ok. + + create_main_release_page(#state{book = Book} = S) -> Panel = wxPanel:new(Book, []), RelBook = wxNotebook:new(Panel, ?wxID_ANY, []), @@ -778,6 +839,9 @@ escript_popup(S, File, Tree, Item) -> handle_event(S, #wx{id = Id, obj= ObjRef, userData = UserData, event = Event} = _Wx) -> %% io:format("wx: ~p\n", [Wx]), case Event of + _ when UserData =:= warnings; + is_tuple(UserData), element(1,UserData)=:=warning -> + handle_warning_event(S, ObjRef, UserData, Event); #wxSize{type = size, size = {W, _H}} when UserData =:= app_list_ctrl -> wxListCtrl:setColumnWidth(ObjRef, ?APPS_APP_COL, W), S; @@ -855,6 +919,78 @@ handle_event(S, #wx{id = Id, obj= ObjRef, userData = UserData, event = Event} = end end. +handle_warning_event(S, ObjRef, _, #wxSize{type = size}) -> + {Total, _} = wxWindow:getClientSize(ObjRef), + SBSize = scroll_size(ObjRef), + wxListCtrl:setColumnWidth(ObjRef, ?WARNING_COL, Total-SBSize), + S; +handle_warning_event(S, ObjRef, _, #wxMouse{type = motion, x=X, y=Y}) -> + Pos = reltool_utils:wait_for_stop_motion(ObjRef, {X,Y}), + Index = wxListCtrl:findItem(ObjRef,-1,Pos,0), + Tip = + case wxListCtrl:getItemText(ObjRef,Index) of + "" -> + ?DEFAULT_WARNING_TIP; + Text -> + "WARNING:\n" ++ Text + end, + wxListCtrl:setToolTip(ObjRef, Tip), + S; +handle_warning_event(S, ObjRef, _, #wxList{type = command_list_item_activated, + itemIndex = Pos}) -> + Text = wxListCtrl:getItemText(ObjRef, Pos), + S#state{warning_wins = [display_warning(S,Text) | S#state.warning_wins]}; +handle_warning_event(S, _ObjRef, {warning,Frame}, + #wxCommand{type = command_button_clicked}) -> + wxFrame:destroy(Frame), + S#state{warning_wins = lists:delete(Frame,S#state.warning_wins)}. + + +display_warning(S,Warning) -> + Pos = warning_popup_position(S,?WARNING_POPUP_SIZE), + Frame = wxFrame:new(wx:null(), ?wxID_ANY, "Warning",[{pos,Pos}]), + Panel = wxPanel:new(Frame, []), + TextStyle = ?wxTE_READONLY bor ?wxTE_WORDWRAP bor ?wxTE_MULTILINE, + Text = wxTextCtrl:new(Panel, ?wxID_ANY, [{value, Warning}, + {style, TextStyle}, + {size, ?WARNING_POPUP_SIZE}]), + Color = wxWindow:getBackgroundColour(Frame), + wxTextCtrl:setBackgroundColour(Text,Color), + Attr = wxTextAttr:new(), + wxTextAttr:setLeftIndent(Attr,10), + wxTextAttr:setRightIndent(Attr,10), + true = wxTextCtrl:setDefaultStyle(Text, Attr), + wxTextAttr:destroy(Attr), + Sizer = wxBoxSizer:new(?wxVERTICAL), + wxSizer:add(Sizer, Text, [{border, 2}, + {flag, ?wxEXPAND bor ?wxALL}, + {proportion, 1}]), + + Close = wxButton:new(Panel, ?wxID_ANY, [{label, "Close"}]), + wxButton:setToolTip(Close, "Close window."), + wxButton:connect(Close, command_button_clicked, [{userData,{warning,Frame}}]), + wxSizer:add(Sizer, Close, [{flag, ?wxALIGN_CENTER_HORIZONTAL}]), + + wxPanel:setSizer(Panel, Sizer), + wxSizer:fit(Sizer, Frame), + wxSizer:setSizeHints(Sizer, Frame), + wxFrame:connect(Frame, close_window), + + wxFrame:show(Frame), + Frame. + +warning_popup_position(#state{frame=MF,warning_list=WL},{WFW,WFH}) -> + {MFX,MFY} = wxWindow:getPosition(MF), + {MFW,MFH} = wxWindow:getSize(MF), + {_WLW,WLH} = wxWindow:getSize(WL), + + %% Position the popup in the middle of the main frame, above the + %% warning list, and with a small offset from the exact middle... + Offset = 50, + X = MFX + (MFW-WFW) div 2 - Offset, + Y = MFY + (MFH-WLH-WFH) div 2 - Offset, + {X,Y}. + handle_popup_event(S, _Type, 0, _ObjRef, _UserData, _Str) -> S#state{popup_menu = undefined}; handle_popup_event(#state{popup_menu = #root_popup{dir = OldDir, @@ -1079,11 +1215,8 @@ handle_app_button(#state{server_pid = ServerPid, app_wins = AppWins} = S, Action) -> NewApps = [move_app(S, Item, Action) || Item <- Items], case reltool_server:set_apps(ServerPid, NewApps) of - {ok, []} -> + {ok, _Warnings} -> ok; - {ok, Warnings} -> - Msg = lists:flatten([[W, $\n] || W <- Warnings]), - display_message(Msg, ?wxICON_WARNING); {error, Reason} -> display_message(Reason, ?wxICON_ERROR) end, @@ -1122,21 +1255,17 @@ move_app(S, {_ItemNo, AppBase}, Action) -> do_set_app(#state{server_pid = ServerPid, app_wins = AppWins} = S, NewApp) -> Result = reltool_server:set_app(ServerPid, NewApp), - [ok = reltool_app_win:refresh(AW#app_win.pid) || AW <- AppWins], - S2 = redraw_apps(S), ReturnApp = case Result of - {ok, AnalysedApp, []} -> - AnalysedApp; - {ok, AnalysedApp, Warnings} -> - Msg = lists:flatten([[W, $\n] || W <- Warnings]), - display_message(Msg, ?wxICON_WARNING), + {ok, AnalysedApp, _Warnings} -> AnalysedApp; {error, Reason} -> display_message(Reason, ?wxICON_ERROR), {ok,OldApp} = reltool_server:get_app(ServerPid, NewApp#app.name), OldApp end, + [ok = reltool_app_win:refresh(AW#app_win.pid) || AW <- AppWins], + S2 = redraw_apps(S), {ok, ReturnApp, S2}. redraw_apps(#state{server_pid = ServerPid, @@ -1159,8 +1288,14 @@ redraw_apps(#state{server_pid = ServerPid, WhiteN = redraw_apps(WhiteApps, WhiteCtrl, ?TICK_IMAGE, ?ERR_IMAGE), redraw_apps(BlackApps2, BlackCtrl, ?CROSS_IMAGE, ?WARN_IMAGE), DerivedN = redraw_apps(DerivedApps, DerivedCtrl, ?TICK_IMAGE, ?ERR_IMAGE), + + WarningsN = redraw_warnings(S), + WarningText = if WarningsN==1 -> "warning"; + true -> "warnings" + end, Status = lists:concat([WhiteN, " whitelisted modules and ", - DerivedN, " derived modules."]), + DerivedN, " derived modules, ", + WarningsN, " ", WarningText, "."]), wxStatusBar:setStatusText(S#state.status_bar, Status), S. @@ -1375,8 +1510,8 @@ check_and_refresh(S, Status) -> case Status of ok -> true; - {ok, Warnings} -> - undo_dialog(S, Warnings); + {ok, _Warnings} -> + true; {error, Reason} when is_list(Reason) -> display_message(Reason, ?wxICON_ERROR), false; @@ -1430,19 +1565,6 @@ question_dialog(Question, Details) -> wxDialog:destroy(Dialog), Answer. -undo_dialog(_S, []) -> - true; -undo_dialog(S, Warnings) -> - Question = "Do you want to perform the update despite these warnings?", - Details = lists:flatten([[W, $\n] || W <- Warnings]), - case question_dialog(Question, Details) of - ?wxID_OK -> - true; - ?wxID_CANCEL -> - ok = reltool_server:undo_config(S#state.server_pid), - false - end. - display_message(Message, Icon) -> Dialog = wxMessageDialog:new(wx:null(), Message, @@ -1486,6 +1608,19 @@ add_text(_,_,[]) -> ok. +scroll_size(ObjRef) -> + case os:type() of + {win32, nt} -> 0; + {unix, darwin} -> + %% I can't figure out is there is a visible scrollbar + %% Always make room for it + wxSystemSettings:getMetric(?wxSYS_VSCROLL_X); + _ -> + case wxWindow:hasScrollbar(ObjRef, ?wxVERTICAL) of + true -> wxSystemSettings:getMetric(?wxSYS_VSCROLL_X); + false -> 0 + end + end. %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% diff --git a/lib/reltool/src/reltool_utils.erl b/lib/reltool/src/reltool_utils.erl index a9107355c7..2d766224d9 100644 --- a/lib/reltool/src/reltool_utils.erl +++ b/lib/reltool/src/reltool_utils.erl @@ -23,7 +23,7 @@ split_app_name/1, prim_consult/1, default_rels/0, choose_default/3, - assign_image_list/1, get_latest_resize/1, + assign_image_list/1, get_latest_resize/1, wait_for_stop_motion/2, mod_conds/0, list_to_mod_cond/1, mod_cond_to_index/1, incl_conds/0, list_to_incl_cond/1, incl_cond_to_index/1, elem_to_index/2, app_dir_test/2, split_app_dir/1, @@ -191,6 +191,16 @@ get_latest_resize(#wx{obj = ObjRef, event = #wxSize{}} = Wx) -> %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% +wait_for_stop_motion(ObjRef, {_,_}=Pos) -> + receive + #wx{obj = ObjRef, event = #wxMouse{type = motion, x=X, y=Y}} -> + wait_for_stop_motion(ObjRef, {X,Y}) + after 100 -> + Pos + end. + +%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% + mod_conds() -> ["all (ebin + app file)", "ebin + derived", "app file + derived", "derived", "none"]. diff --git a/lib/reltool/test/reltool_manual_gui_SUITE.erl b/lib/reltool/test/reltool_manual_gui_SUITE.erl index c6b1d56988..1ebee9fae1 100644 --- a/lib/reltool/test/reltool_manual_gui_SUITE.erl +++ b/lib/reltool/test/reltool_manual_gui_SUITE.erl @@ -67,37 +67,49 @@ config(Config) -> SimpleConfigFile = create_simple_config(PrivDir), WarningConfigFile = create_warning_config(PrivDir,DataDir), - break("there are no modules in the 'Included' and 'Excluded' columns", + break("there are no modules in the 'Included' and 'Excluded' columns, " + "and now warnings are displayed", {"load configuration ~p",[SimpleConfigFile]}), break("kernel, stdlib and sasl are included and all other are excluded", "undo"), break("we are back to default - no included and no excluded applications", "undo again"), break("kernel, stdlib and sasl are included and all other are excluded", - {"load configuration ~p, but click 'cancel' in the warning dialog", + {"load configuration ~p", [WarningConfigFile]}), - break("no change is done", - "load same configuration again and this time click 'ok' in the dialog"), - break("application a is added in the 'Included' column", + break("a warning is displayed in the warning list", + "undo"), + break("no warning is displayed in the warning list", + "load same configuration again"), + break("application a is added in the 'Included' column and " + "one warning is displayed", "reset configuration"), - break("we are back to default - no included and no excluded applications", + break("we are back to default - no included and no excluded applications, " + "and no warnings", "undo"), - break("a warning dialog is displayed, with only an ok button", - "click ok"), - break("a, kernel, stdlib and sasl are included and all other are excluded", + break("a, kernel, stdlib and sasl are included and all other are excluded. " + "One warning should now exist through the rest of this test case", + "double click the warning"), + break("a popup window occurs displaying the warning text", + "close it with the 'Close' button"), + break("it disappears", + "open it again"), + break("the warning text can be marked, copied and pasted", + "close the popup with the close box on the top frame"), + break("it disappears", "select application a from 'Included' column and click red cross to " "exclude it"), break("application a is moved to 'Excluded' column", "select application tools from 'Excluded' column and click green V to " "include it"), break("application tools is moved to 'Included' column", - "select application runtime-tools from 'Excluded' column and click " + "select application runtime_tools from 'Excluded' column and click " "green V to include it"), - break("application runtime-tools is moved to 'Included' column", + break("application runtime_tools is moved to 'Included' column", "undo"), ExplicitConfig = filename:join(PrivDir,"explicit.config"), - break("application runtime-tools is moved back to 'Excluded' column", + break("application runtime_tools is moved back to 'Excluded' column", {"save configuration as 'explicit' to ~p",[ExplicitConfig]}), ExpectedExplicitConfig = {sys,[{lib_dirs,[filename:join(DataDir,"faulty_app_file")]}, @@ -113,7 +125,7 @@ config(Config) -> "Now go to the 'Libraries' tab and change the root directory to " "some invalid directory."), break("an error dialog occurs saying that there is no lib dir", - {"add library directory ~p, and click 'ok' in warning dialog", + {"add library directory ~p", [filename:join(DataDir,"dependencies")]}), break("applications x, y and z are added to the 'Excluded' column in " "'Applications' tab", @@ -121,7 +133,7 @@ config(Config) -> "to 'derived'"), break("a is excluded, kernel, stdlib, sasl and tools are included and " "x, y and z are available in the 'Applications' tab", - "undo, and click ok in the warning dialog"), + "undo"), break("all non included application are moved to the 'Excluded' column " "and that 'Application inclusion policy' under 'System settings' " "tab is set back to 'exclude'", @@ -218,7 +230,13 @@ break(Check,Do) -> break(CheckStr,DoStr). break(Str) -> - test_server:break(Str). + Count = + case get(count) of + undefined -> 1; + C -> C + end, + put(count,Count+1), + test_server:break("Step " ++ integer_to_list(Count) ++ ":\n" ++ Str). check_config(Expected,File) -> {ok,[Config]} = file:consult(File), -- cgit v1.2.3