microsoft / microsoft/terminal

InternalActionID is a hash, which cannot be depended on for equality comparison

Open
#13,124 2 comments 0 reactions 0 assignees View on GitHub
Area-Settings Issue-Bug Product-Terminal
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.