microsoft / microsoft/terminal
InternalActionID is a hash, which cannot be depended on for equality comparison
- Dominant language
- C++
- Stars
- 105k
- Forks
- 9.6k
- Avg merge
- 3d 17h
- Merged PRs (30d)
- 29
Description
### Windows Terminal version
latest
### Windows build number
10.0.22621.0
### Other Software
_No response_
### Steps to reproduce
From `doc\specs\#885 - Terminal Settings Model\Actions Addendum.md`:
```c++
std::map _KeyMap;
std::map _ActionMap;
```
> `InternalActionID` will be a hash of `ActionAndArgs` such that two `ActionAndArgs` with the same `ShortcutAction` and `IActionArgs` output the same hash value.
A hash function can be used to quickly determine if two items are *NOT* equal: if they have different hashes, they are not the same.
However, if the hashes are the same, *that does not mean the items are equal*. It just means that you've had a "hash collision", and need to perform a thorough equality check. But Terminal uses `InternalActionID` as a dictionary key (and the values are not lists), so if you have a collision, an action gets dropped.
I ran into this trying to add a new action--the hash function was not great, so there were collisions, which resulted in very confusing behavior, since some of my actions appeared to be straight-up do the wrong thing, but not just *any* wrong thing; but the same thing as some other instance of the command with different arguments!
I was fortunately able to improve the hash function, but this still depends on getting lucky.
### Expected Behavior
I expected to be able to add a new ActionAndArgs, even though it had a not-very-good hash function, without some of my actions being mysteriously subsumed by others.
### Actual Behavior
Some new actions appeared to do the wrong thing (the same thing as some other action that had different args, but ended up with the same hash value).
Contributor guide
Research direction
Start with doc\specs\#885 - Terminal Settings Model\Actions Addendum.md and trace the InternalActionID, ActionAndArgs, _KeyMap, and _ActionMap entry points it describes. Reproduce the collision scenario and inspect how distinct actions are stored and retrieved. Done means actions with different arguments no longer get silently replaced when their hashes collide.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- cli
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100