Code review comment for lp:~simon-kersey/bzr-explorer/add-tool-cmd-selected-support

Revision history for this message
Simon Kersey (simon-kersey) wrote :

I have not had much time to progress this.
I have some reservations about landing it. Mainly due to reconsidering the last fix that I put in.
I think that I should go back to the original idea I had which was to introduce a new token that contained whatever was selected in the working tree (e.g. %wt_selected%). The problem was caused by %selected% being used by exisiting code to represent something else (in this case I think it was what was selected in a previous screen)so that when the log button was pressed you got a log for the selected branch. The other issue is knowing what %selected% actually means. As you make changes should %selected% represent the items modified etc. What I wanted to implement was a simple way to allow tools to use what was selected in the working tree so that plugins could provide a richer interface.
If you think that overloading the %selected% token (so that, when used in tool definitions, the token is replaced by what is selected in the working tree or, if nothing is selected in the working tree, left with the existing content) is OK then it should be OK to merge in and I will put up some documentation updates in separate patch.
I think my preference is to use a new token for the avoidance of doubt about behaviour.
Any thoughts?

« Back to merge proposal