From: Eugene S. <eug...@gm...> - 2013-03-28 11:46:36
|
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/109777/ ----------------------------------------------------------- Review request for Kile and Michel Ludwig. Description ------- As per discussion in the mail list. Do not add "tools_.+" actions to the actions collection. Try to get action which is not found in the action collection from the current view Diffs ----- src/scripting/kilescriptdocument.cpp 7c5dfde src/scriptmanager.cpp 2687794 Diff: http://git.reviewboard.kde.org/r/109777/diff/ Testing ------- manual Thanks, Eugene Shalygin |
From: Michel L. <mic...@gm...> - 2013-03-28 12:03:07
|
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/109777/#review29996 ----------------------------------------------------------- Thanks Eugene. I think it would even be better if "triggerAction" wouldn't have to be called at all. Could you maybe replace the call to "triggerAction" with the respective QAction *action = m_view->action("bla"); if(action) { action->trigger(); } in "kilescriptdocument.cpp"? (you could also introduce a method for just doing that) I guess the same could be done for the actions that belong to Kile, even by calling the methods directy via KileInfo, and then we could remove "Manager::initScriptActions()" altogether in "scriptmanager.cpp". - Michel Ludwig On March 28, 2013, 11:46 a.m., Eugene Shalygin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/109777/ > ----------------------------------------------------------- > > (Updated March 28, 2013, 11:46 a.m.) > > > Review request for Kile and Michel Ludwig. > > > Description > ------- > > As per discussion in the mail list. > > Do not add "tools_.+" actions to the actions collection. Try to get action which is not found in the action collection from the current view > > > Diffs > ----- > > src/scripting/kilescriptdocument.cpp 7c5dfde > src/scriptmanager.cpp 2687794 > > Diff: http://git.reviewboard.kde.org/r/109777/diff/ > > > Testing > ------- > > manual > > > Thanks, > > Eugene Shalygin > > |
From: Eugene S. <eug...@gm...> - 2013-03-28 17:34:11
|
> On March 28, 2013, 12:02 p.m., Michel Ludwig wrote: > > Thanks Eugene. > > > > I think it would even be better if "triggerAction" wouldn't have to be called at all. Could you maybe replace the call to "triggerAction" with the respective > > > > QAction *action = m_view->action("bla"); > > if(action) { > > action->trigger(); > > } > > > > in "kilescriptdocument.cpp"? (you could also introduce a method for just doing that) > > > > I guess the same could be done for the actions that belong to Kile, even by calling the methods directy via KileInfo, and then we could remove "Manager::initScriptActions()" > > altogether in "scriptmanager.cpp". > > Michel, could you share your thought on the following, please? If we are going to remove action objects then I see two possible obstacles. 1. Some of the actions are bound to Kile class methods, so instead of KileInfo we would need Kile object pointer. 2. Some of the actions are using EditorExtensions::insertTag and there is some set-up code present for XMLGUI part. Will it make sense to duplicate this code in KileScriptDocument? - Eugene ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/109777/#review29996 ----------------------------------------------------------- On March 28, 2013, 11:46 a.m., Eugene Shalygin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/109777/ > ----------------------------------------------------------- > > (Updated March 28, 2013, 11:46 a.m.) > > > Review request for Kile and Michel Ludwig. > > > Description > ------- > > As per discussion in the mail list. > > Do not add "tools_.+" actions to the actions collection. Try to get action which is not found in the action collection from the current view > > > Diffs > ----- > > src/scripting/kilescriptdocument.cpp 7c5dfde > src/scriptmanager.cpp 2687794 > > Diff: http://git.reviewboard.kde.org/r/109777/diff/ > > > Testing > ------- > > manual > > > Thanks, > > Eugene Shalygin > > |
From: Michel L. <mic...@gm...> - 2013-03-31 20:05:48
|
> On March 28, 2013, 12:02 p.m., Michel Ludwig wrote: > > Thanks Eugene. > > > > I think it would even be better if "triggerAction" wouldn't have to be called at all. Could you maybe replace the call to "triggerAction" with the respective > > > > QAction *action = m_view->action("bla"); > > if(action) { > > action->trigger(); > > } > > > > in "kilescriptdocument.cpp"? (you could also introduce a method for just doing that) > > > > I guess the same could be done for the actions that belong to Kile, even by calling the methods directy via KileInfo, and then we could remove "Manager::initScriptActions()" > > altogether in "scriptmanager.cpp". > > > > Eugene Shalygin wrote: > Michel, > > could you share your thought on the following, please? If we are going to remove action objects then I see two possible obstacles. 1. Some of the actions are bound to Kile class methods, so instead of KileInfo we would need Kile object pointer. 2. Some of the actions are using EditorExtensions::insertTag and there is some set-up code present for XMLGUI part. Will it make sense to duplicate this code in KileScriptDocument? The main goal should be to remove the action name -> QAction* map. Maybe just see what is easier: if it's easy to use a method from KileInfo directly, then just do that; otherwise the corresponding action will have to be triggered. - Michel ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/109777/#review29996 ----------------------------------------------------------- On March 28, 2013, 11:46 a.m., Eugene Shalygin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/109777/ > ----------------------------------------------------------- > > (Updated March 28, 2013, 11:46 a.m.) > > > Review request for Kile and Michel Ludwig. > > > Description > ------- > > As per discussion in the mail list. > > Do not add "tools_.+" actions to the actions collection. Try to get action which is not found in the action collection from the current view > > > Diffs > ----- > > src/scripting/kilescriptdocument.cpp 7c5dfde > src/scriptmanager.cpp 2687794 > > Diff: http://git.reviewboard.kde.org/r/109777/diff/ > > > Testing > ------- > > manual > > > Thanks, > > Eugene Shalygin > > |