Opened 6 years ago
Closed 5 years ago
#2600 closed defect (fixed)
Problem with "toolshed show"
Reported by: | pett | Owned by: | Greg Couch |
---|---|---|---|
Priority: | moderate | Milestone: | 1.0 |
Component: | Tool Shed | Version: | 1.0 |
Keywords: | Cc: | chimera-staff@… | |
Blocked By: | Blocking: | ||
Notify when closed: | Platform: | all | |
Project: | ChimeraX |
Description
The first thing "toolshed show" does is to look for existing matching instances of the tool name to call display() on. One of the steps of that search is to match the tool name against the bundle name. This is problematic if a bundle registers multiple tools, one of which happens to match the bundle name.
For instance, my "Clashes" bundle register two tools: Clashes and Contacts. The bundle-name-matching code means that I can start "Clashes" and then "Contacts" okay, but not "Contacts" and then "Clashes" because when trying to start "Clashes" that code sees that the bundle name for "Contacts" is "Clashes" and just displays "Contacts" again instead of starting "Clashes".
Perhaps that code should be nuked entirely, but for now I have just commented it out pending your review.
Greg also points out that the tool-name expansion that ts.find_bundle_for_tool() performs probably should occur earlier, before the possibly shortened original tool name is compared against various values.
Change History (11)
comment:1 by , 6 years ago
Cc: | added; removed |
---|
follow-up: 2 comment:2 by , 6 years ago
Maybe the solution to starting a new instance of a tool when one already exists is to just have an additional option ui show MyTool new true
comment:3 by , 6 years ago
Well, the Tools menu uses "toolshed show" when you choose a menu item. It doesn't know if it should be adding "new true" or not. I suppose it could rummage around the running tools to try to find out, but then it would be doing the same work that "toolshed show" is doing now.
I think the logic needs to be:
1) Match running tools whose display [then tool] name exactly matches
2) Match non-running tools whose name exactly matches
3) Use "starts with" match against running tools
4) Use "starts with" match against non-running tools
Multiple matches shows all running tools, raises an error for non-running tools.
comment:4 by , 6 years ago
That's essentially what I described except your (3) and (4) are prioritized and mine are lumped together. Yours has a nice and simple description for the algorithm, so I'll work with that unless/until someone objects.
comment:5 by , 6 years ago
If it turns out that we need to limit "toolshed show" to one of those steps, we can add keyword options later. But without keywords, it does all of those steps.
comment:6 by , 6 years ago
Just to be clear, if there are exact matches, then we do not do the "start with" checks. Is that correct?
comment:8 by , 6 years ago
7050f598b9 is my first cut at the new semantics. Please try it and let me know if it works as expected.
The tool name is now case-independent. As described above, the "tool name" can now be a prefix. So "toolshed show basic" should now bring up the "Basic Actions" tool.
comment:9 by , 5 years ago
Milestone: | → 1.0 |
---|
comment:10 by , 5 years ago
Owner: | changed from | to
---|
Improve error message when there are multiple matches to list the possibilities.
What is the semantics we're going for with "toolshed show" (to be "ui show")? The current behavior seems wrong:
I propose (after brief discussion with Eric and Greg) that we change it to:
The down side that I can think of with this scheme is that the user cannot start a second copy of a registered tool if its instances do not change their display name. (If they do not change their names, they will all have the same tool name, and the user always match a running instance and therefore never fall through to starting another instance of the tool.)
Comments?