Skip to content
New issue

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

#

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

Already on GitHub? # to your account

Jump to bottom

Command History (F7) is added to when selected command from the list #17797

Open
MogHughson opened this issue Aug 24, 2024 · 2 comments · Fixed by #17852
Open

Command History (F7) is added to when selected command from the list #17797

MogHughson opened this issue Aug 24, 2024 · 2 comments · Fixed by #17852
Assignees
@lhecker
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
Milestone
Terminal v1.23

Comments

@MogHughson
Copy link

Windows Terminal version

1.20.11781.0

Windows build number

10.0.22631.4037

Other Software

No response

Steps to reproduce

  1. Open a clean command prompt - nothing in the list in F7.
  2. Type in a command, e.g. "dir"
  3. Type in a second different command, e.g. "dir D*"
  4. Press F7 to see the command list - it will contain two commands. Select the first one from the list.
  5. Press F7 to see the command list again, it will now contain three commands. This is unexpected.

Expected Behavior

Selecting the command from the command list was previously (in Windows 10 command prompt) the way you issued it to ensure it didn't get added to the history, thus allowing you to maintain a nice tidy command list.

Actual Behavior

Instead it was added to the history.
@MogHughson MogHughson added Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Aug 24, 2024
@zadjii-msft zadjii-msft added Product-Conhost For issues in the Console codebase Area-CookedRead The cmd.exe COOKED_READ handling labels Aug 26, 2024
@zadjii-msft zadjii-msft added this to the Terminal v1.23 milestone Aug 26, 2024
@github-project-automation github-project-automation bot moved this to To Cherry Pick in 1.21 Servicing Pipeline Aug 26, 2024
@github-project-automation github-project-automation bot moved this to To Cherry Pick in 1.22 Servicing Pipeline Aug 26, 2024
@zadjii-msft zadjii-msft removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Aug 28, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Tag-Fix Doesn't match tag requirements label Aug 28, 2024
@lhecker lhecker mentioned this issue Sep 3, 2024
Merged
@microsoft-github-policy-service microsoft-github-policy-service bot added the In-PR This issue has a related PR label Sep 3, 2024
@DHowett DHowett closed this as completed in 5fdfd51 Sep 4, 2024
@DHowett DHowett moved this from To Cherry Pick to Cherry Picked in 1.21 Servicing Pipeline Sep 4, 2024
DHowett pushed a commit that referenced this issue Sep 4, 2024
@lhecker @DHowett
Dedup command history by default (#17852)
c47d9e1 
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 DHowett moved this from To Cherry Pick to Cherry Picked in 1.22 Servicing Pipeline Sep 4, 2024
DHowett pushed a commit that referenced this issue Sep 4, 2024
@lhecker @DHowett
Dedup command history by default (#17852)
03fc325 
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
@DHowett DHowett moved this from Cherry Picked to Shipped in 1.21 Servicing Pipeline Oct 2, 2024
@DHowett DHowett moved this from Cherry Picked to Shipped in 1.22 Servicing Pipeline Oct 2, 2024
@lhecker lhecker mentioned this issue Nov 21, 2024
Merged
@lhecker lhecker removed their assignment Nov 21, 2024
@lhecker lhecker added Needs-Discussion Something that requires a team discussion before we can proceed and removed In-PR This issue has a related PR labels Nov 21, 2024
@lhecker
Copy link
Member

lhecker commented Nov 21, 2024

As per #18138 I've decided to revert this change for now.

@lhecker lhecker reopened this Nov 21, 2024
@carlos-zamora
Copy link
Member

Discussed a bit as a team. Some notes:

  • Need to look at this case a little more. Intersection of command history popup and error navigation.
  • from Command History (F7) is added to when selected command from the list #17797:
    • " Expected Behavior: Selecting the command from the command list was previously (in Windows 10 command prompt) the way you issued it to ensure it didn't get added to the history, thus allowing you to maintain a nice tidy command list."
    • suggests it's not about the deduplication itself, more just that the workflow is a little different.

@lhecker lhecker removed Needs-Tag-Fix Doesn't match tag requirements Needs-Discussion Something that requires a team discussion before we can proceed labels Nov 25, 2024
@lhecker lhecker self-assigned this Nov 25, 2024
DHowett pushed a commit that referenced this issue Nov 25, 2024
@lhecker
Revert "Dedup command history by default (#17852)" (#18229)
220c7cd 
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
DHowett pushed a commit that referenced this issue Feb 3, 2025
@lhecker @DHowett
Revert "Dedup command history by default (#17852)" (#18229)
938b219 
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

(cherry picked from commit 220c7cd)
Service-Card-Id: PVTI_lADOAF3p4s4AmhmQzgUeWb8
Service-Version: 1.22
# for free to join this conversation on GitHub. Already have an account? # to comment
Assignees

@lhecker lhecker

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
1.21 Servicing Pipeline
Status: Shipped
1.22 Servicing Pipeline
Status: Shipped
Milestone
Terminal v1.23
Development

Successfully merging a pull request may close this issue.

Dedup command history by default microsoft/terminal
4 participants
@lhecker @carlos-zamora @zadjii-msft @MogHughson