Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dedup command history by default #17852

Merged
merged 2 commits into from
Sep 4, 2024
Merged

Conversation

lhecker
Copy link
Member

@lhecker lhecker commented Sep 3, 2024

Under ConPTY we don't load any user settings. SetUpConsole notes:

If we are [ConPTY], we don't want to load any user settings,
because that could result in some strange rendering results [...]

This enables deduplication by default, which I figured wouldn't cause
any regressions since it's a user-controllable setting anyway, while
it's clearly something the average user wants enabled, for the same
reason that PSReadLine has HistoryNoDuplicates enabled by default.

Closes #17797

Validation Steps Performed

  • Launch conhost, enter 2 commands, press F7, select the older one,
    press Enter, press F7. 2 entries ✅
  • Launch WT, enter 2 commands, press F7, select the older one,
    press Enter, press F7. 2 entries ✅

@microsoft-github-policy-service microsoft-github-policy-service bot added Issue-Bug It either shouldn't be doing this or needs an investigation. Area-CookedRead The cmd.exe COOKED_READ handling Product-Conhost For issues in the Console codebase labels Sep 3, 2024
Copy link
Member

@DHowett DHowett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm OK with this!

@DHowett DHowett merged commit 5fdfd51 into main Sep 4, 2024
20 checks passed
@DHowett DHowett deleted the dev/lhecker/17797-dedup-by-default branch September 4, 2024 19:57
DHowett pushed a commit that referenced this pull request Sep 4, 2024
Under ConPTY we don't load any user settings. `SetUpConsole` notes:
> If we are [ConPTY], we don't want to load any user settings,
> because that could result in some strange rendering results [...]

This enables deduplication by default, which I figured wouldn't cause
any regressions since it's a user-controllable setting anyway, while
it's clearly something the average user wants enabled, for the same
reason that PSReadLine has HistoryNoDuplicates enabled by default.

Closes #17797

## Validation Steps Performed
* Launch conhost, enter 2 commands, press F7, select the older one,
  press Enter, press F7. 2 entries ✅
* Launch WT, enter 2 commands, press F7, select the older one,
  press Enter, press F7. 2 entries ✅

(cherry picked from commit 5fdfd51)
Service-Card-Id: PVTI_lADOAF3p4s4AmhmszgSN1FM
Service-Version: 1.21
DHowett pushed a commit that referenced this pull request Sep 4, 2024
Under ConPTY we don't load any user settings. `SetUpConsole` notes:
> If we are [ConPTY], we don't want to load any user settings,
> because that could result in some strange rendering results [...]

This enables deduplication by default, which I figured wouldn't cause
any regressions since it's a user-controllable setting anyway, while
it's clearly something the average user wants enabled, for the same
reason that PSReadLine has HistoryNoDuplicates enabled by default.

Closes #17797

## Validation Steps Performed
* Launch conhost, enter 2 commands, press F7, select the older one,
  press Enter, press F7. 2 entries ✅
* Launch WT, enter 2 commands, press F7, select the older one,
  press Enter, press F7. 2 entries ✅

(cherry picked from commit 5fdfd51)
Service-Card-Id: PVTI_lADOAF3p4s4AmhmQzgSN1FQ
Service-Version: 1.22
@ikkierie
Copy link

ikkierie commented Oct 28, 2024

Hi, sorry if this is the wrong place to ask but, uh, how do you disable this change?

I have HistoryNoDup set in the registry and CMD.exe respects it but windows terminal doesn't seem to.

I hadn't noticed before this change was made, but now it means my history isn't working properly lol. (Deduping the history makes it a lot less useful for me, as you can't go back to a previous command and press down+enter repeatedly to replay the history sequence, which is very useful in applications that use the default console line input).

@lhecker lhecker added the Needs-Discussion Something that requires a team discussion before we can proceed label Nov 1, 2024
@lhecker lhecker removed the Needs-Discussion Something that requires a team discussion before we can proceed label Nov 18, 2024
lhecker added a commit that referenced this pull request Nov 21, 2024
DHowett pushed a commit that referenced this pull request Nov 25, 2024
This reverts commit 5fdfd51,
because 3 people complained about this change VS 1 person
requesting the change to be made in the first place.

Closes #18138
Reopens #17797 for discussion
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-CookedRead The cmd.exe COOKED_READ handling Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Conhost For issues in the Console codebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Command History (F7) is added to when selected command from the list
4 participants