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

Add support for local snippets in the CWD #17388

Merged
merged 20 commits into from
Jul 26, 2024

Conversation

zadjii-msft
Copy link
Member

This PR adds the ability to load snippets from the CWD into the suggestions UI.

If shell integration is disabled, then we only ever think the CWD for a pane is it's startingDirectory. So, in the default case, users can still stick snippets into the root of their git repos, and have the Terminal load them automatically (for profiles starting in the root of their repo).
If it's enabled though, we'll always try to load snippets from the CWD of the shell.

  • We cache the actions into a separate map of CWD -> actions. This lets us read the file only the first time we see a dir.
  • We clear that cache on settings reload
  • We only load sendInput actions from the .wt.json

As spec'd in #17329

commit deeb281708acf66150f3ad519b50a475605670de
Author: Mike Griese <[email protected]>
Date:   Fri Jun 7 06:11:21 2024 -0500

    format

commit deab3fe
Merge: eb750cc aeed078
Author: Mike Griese <[email protected]>
Date:   Fri Jun 7 05:24:14 2024 -0500

    Merge remote-tracking branch 'origin/main' into dev/migrie/f/local-snippets-cleaner

commit eb750cc
Merge: f0bc7f1 ab65ecb
Author: Mike Griese <[email protected]>
Date:   Fri Jun 7 05:21:24 2024 -0500

    Merge remote-tracking branch 'origin/dev/pabhoj/action_new_tab_menu' into dev/migrie/f/local-snippets-cleaner

commit ab65ecb
Merge: 0f7e9f3 cc837b6
Author: Pankaj Bhojwani <[email protected]>
Date:   Tue Jun 4 13:06:42 2024 -0700

    Merge branch 'dev/pabhoj/command_keys' into dev/pabhoj/action_new_tab_menu

commit cc837b6
Author: Pankaj Bhojwani <[email protected]>
Date:   Tue Jun 4 13:05:53 2024 -0700

    fix test

commit 0f7e9f3
Author: Pankaj Bhojwani <[email protected]>
Date:   Tue Jun 4 12:50:15 2024 -0700

    fixes

commit 589a1e0
Author: Pankaj Bhojwani <[email protected]>
Date:   Tue Jun 4 11:29:23 2024 -0700

    IDTo

commit 6ea25e4
Author: Pankaj Bhojwani <[email protected]>
Date:   Tue Jun 4 11:24:24 2024 -0700

    ID

commit 258c6eb
Author: Pankaj Bhojwani <[email protected]>
Date:   Tue Jun 4 11:19:49 2024 -0700

    O(1), rename maps for clarity

commit 327858b
Merge: 5a00d5f 96d8d1f
Author: Pankaj Bhojwani <[email protected]>
Date:   Tue Jun 4 10:20:27 2024 -0700

    Merge branch 'dev/pabhoj/action_refactor' into dev/pabhoj/command_keys

commit 96d8d1f
Merge: 406312f ece0c04
Author: Pankaj Bhojwani <[email protected]>
Date:   Tue Jun 4 10:19:47 2024 -0700

    Merge branch 'main' of https://github.com/microsoft/terminal into dev/pabhoj/action_refactor

commit 5a00d5f
Author: Pankaj Bhojwani <[email protected]>
Date:   Mon Jun 3 16:13:37 2024 -0700

    nits and changes

commit 253dedf
Merge: d4d216c 406312f
Author: Pankaj Bhojwani <[email protected]>
Date:   Mon Jun 3 11:31:47 2024 -0700

    conflict

commit f0bc7f1
Author: Mike Griese <[email protected]>
Date:   Sat Jun 1 21:42:03 2024 -0500

    last cleanup before review

commit 3aa5b4b
Author: Mike Griese <[email protected]>
Date:   Sat Jun 1 21:19:33 2024 -0500

    more cleanup

commit 091797c
Author: Mike Griese <[email protected]>
Date:   Sat Jun 1 21:03:58 2024 -0500

    cleanup

commit d41e973
Author: Mike Griese <[email protected]>
Date:   Sat Jun 1 20:44:47 2024 -0500

    a much cleaner abstraction

commit 5a029f0
Author: Mike Griese <[email protected]>
Date:   Sat Jun 1 20:11:15 2024 -0500

    smaller refactors

commit bba8aac
Author: Mike Griese <[email protected]>
Date:   Sat Jun 1 14:37:50 2024 -0500

    use a cache in actionmap to stash commands as we parse them

commit 5466965
Author: Mike Griese <[email protected]>
Date:   Sat Jun 1 13:52:38 2024 -0500

    Actually works in suggestions UI again

commit b5d063b
Author: Mike Griese <[email protected]>
Date:   Sat Jun 1 12:27:21 2024 -0500

    plumb it through. It works

      (cherry-picked from 3899919)

commit 406312f
Author: Pankaj Bhojwani <[email protected]>
Date:   Fri May 31 19:33:07 2024 -0700

    fix loops

commit 625753c
Author: Pankaj Bhojwani <[email protected]>
Date:   Fri May 31 17:52:20 2024 -0700

    x86 hash

commit a80316d
Author: Pankaj Bhojwani <[email protected]>
Date:   Fri May 31 16:56:34 2024 -0700

    1 more test

commit 9703815
Author: Pankaj Bhojwani <[email protected]>
Date:   Fri May 31 16:32:21 2024 -0700

    nits n fixes

commit 0dff336
Author: Pankaj Bhojwani <[email protected]>
Date:   Fri May 31 10:51:36 2024 -0700

    nits, schema

commit b88a8c5
Author: Pankaj Bhojwani <[email protected]>
Date:   Thu May 30 13:03:03 2024 -0700

    eraseif

commit 6c6dd46
Author: Pankaj Bhojwani <[email protected]>
Date:   Mon May 20 15:06:31 2024 -0700

    leonard comments

commit 51528e9
Author: Pankaj Bhojwani <[email protected]>
Date:   Thu May 16 16:59:35 2024 -0700

    spaces

commit 40b4aa2
Author: Pankaj Bhojwani <[email protected]>
Date:   Thu May 16 16:50:56 2024 -0700

    works

commit d4d216c
Author: Pankaj Bhojwani <[email protected]>
Date:   Wed May 8 10:48:33 2024 -0700

    don't need helper anymore

commit 7d00b25
Author: Pankaj Bhojwani <[email protected]>
Date:   Wed May 8 10:11:16 2024 -0700

    spelling

commit ba375ec
Author: Pankaj Bhojwani <[email protected]>
Date:   Wed May 8 10:00:45 2024 -0700

    remove keys from command

commit 5e48a45
Author: Pankaj Bhojwani <[email protected]>
Date:   Tue May 7 15:56:52 2024 -0700

    update add action

commit 14d83b5
Author: Pankaj Bhojwani <[email protected]>
Date:   Mon May 6 16:50:01 2024 -0700

    delete user actions that are identical to inbox actions

commit 3e31bda
Author: Pankaj Bhojwani <[email protected]>
Date:   Mon May 6 15:29:24 2024 -0700

    generate here instead

commit 4d35c14
Merge: abef25d 3996806
Author: Pankaj Bhojwani <[email protected]>
Date:   Fri May 3 14:54:09 2024 -0700

    schema conflict

commit abef25d
Author: Pankaj Bhojwani <[email protected]>
Date:   Fri May 3 14:51:51 2024 -0700

    move this to header

commit 7793c5c
Author: Pankaj Bhojwani <[email protected]>
Date:   Fri May 3 13:30:09 2024 -0700

    schema

commit ccf1cc9
Author: Pankaj Bhojwani <[email protected]>
Date:   Thu May 2 19:06:51 2024 -0700

    nits

commit ebc03e9
Author: Pankaj Bhojwani <[email protected]>
Date:   Thu May 2 18:44:44 2024 -0700

    another test

commit 80fc299
Author: Pankaj Bhojwani <[email protected]>
Date:   Thu May 2 18:11:46 2024 -0700

    some new tests

commit 02a1e37
Author: Pankaj Bhojwani <[email protected]>
Date:   Wed May 1 18:05:06 2024 -0700

    correct GH todo

commit 0480d65
Author: Pankaj Bhojwani <[email protected]>
Date:   Wed May 1 17:54:46 2024 -0700

    this is better

commit 193e573
Author: Pankaj Bhojwani <[email protected]>
Date:   Wed May 1 17:36:31 2024 -0700

    fix remaining tests

commit 2b16acd
Author: Pankaj Bhojwani <[email protected]>
Date:   Wed May 1 14:48:37 2024 -0700

    check for name, fix some tests

commit f35bf20
Merge: cdb907d d380394
Author: Pankaj Bhojwani <[email protected]>
Date:   Wed May 1 10:50:02 2024 -0700

    Merge branch 'main' of https://github.com/microsoft/terminal into dev/pabhoj/action_refactor

commit cdb907d
Author: Pankaj Bhojwani <[email protected]>
Date:   Tue Apr 30 17:43:25 2024 -0700

    mark todo

commit 3e601f5
Author: Pankaj Bhojwani <[email protected]>
Date:   Tue Apr 30 17:36:37 2024 -0700

    better if

commit c2c75c8
Author: Pankaj Bhojwani <[email protected]>
Date:   Tue Apr 30 17:33:53 2024 -0700

    bandaid temporary fix for name

commit 3c6015d
Author: Pankaj Bhojwani <[email protected]>
Date:   Tue Apr 30 16:45:59 2024 -0700

    remove _getcumulativeactions

commit 45cfcd6
Author: Pankaj Bhojwani <[email protected]>
Date:   Tue Apr 30 15:24:33 2024 -0700

    just add duplicate pane auto to defaults

commit ca4015f
Author: Pankaj Bhojwani <[email protected]>
Date:   Tue Apr 30 15:19:33 2024 -0700

    only one tojson

commit 4c744e6
Author: Pankaj Bhojwani <[email protected]>
Date:   Tue Apr 30 14:56:42 2024 -0700

    misc

commit 6437b9f
Author: Pankaj Bhojwani <[email protected]>
Date:   Tue Apr 30 14:36:09 2024 -0700

    fix user defaults file

commit 428821b
Author: Pankaj Bhojwani <[email protected]>
Date:   Tue Apr 30 13:04:14 2024 -0700

    remove _idwasgenerated

commit 3d92f27
Author: Pankaj Bhojwani <[email protected]>
Date:   Tue Apr 30 11:18:12 2024 -0700

    format

commit db00b90
Author: Pankaj Bhojwani <[email protected]>
Date:   Tue Apr 30 11:14:21 2024 -0700

    spelling things

commit e725f1e
Merge: e62dfa2 6bc7b9e
Author: Pankaj Bhojwani <[email protected]>
Date:   Tue Apr 30 11:09:51 2024 -0700

    resolve conflict

commit e62dfa2
Author: Pankaj Bhojwani <[email protected]>
Date:   Tue Apr 30 11:09:15 2024 -0700

    some comments

commit 2b4aeb2
Author: Pankaj Bhojwani <[email protected]>
Date:   Tue Apr 30 11:04:01 2024 -0700

    don't check for special in standard

commit 2f1d8d2
Author: Pankaj Bhojwani <[email protected]>
Date:   Tue Apr 30 10:34:58 2024 -0700

    update defaults

commit 754bf04
Author: Pankaj Bhojwani <[email protected]>
Date:   Tue Apr 30 10:27:25 2024 -0700

    mark gh todo

commit c51558f
Author: Pankaj Bhojwani <[email protected]>
Date:   Tue Apr 30 10:20:07 2024 -0700

    unmark these

commit 5a1b822
Author: Pankaj Bhojwani <[email protected]>
Date:   Mon Apr 29 21:33:33 2024 -0700

    reimplement populating all known keybindings

commit b3e9c26
Author: Pankaj Bhojwani <[email protected]>
Date:   Sat Apr 27 16:22:14 2024 -0700

    remove check for invalid

commit 936afd6
Author: Pankaj Bhojwani <[email protected]>
Date:   Sat Apr 27 16:16:25 2024 -0700

    _getactionbyid no longer returns optional

commit dc874c3
Author: Pankaj Bhojwani <[email protected]>
Date:   Sat Apr 27 15:36:17 2024 -0700

    rename to special/standard

commit ae16a5e
Author: Pankaj Bhojwani <[email protected]>
Date:   Fri Apr 26 15:43:06 2024 -0700

    started stage 3

commit 3e7ab38
Author: Pankaj Bhojwani <[email protected]>
Date:   Fri Apr 26 11:26:10 2024 -0700

    sui works?

commit ddfac90
Merge: f1633e0 41bb28c
Author: Pankaj Bhojwani <[email protected]>
Date:   Fri Apr 26 09:56:46 2024 -0700

    Merge branch 'main' of https://github.com/microsoft/terminal into dev/pabhoj/action_refactor

commit f1633e0
Author: Pankaj Bhojwani <[email protected]>
Date:   Thu Apr 25 21:25:17 2024 -0700

    overwritten IDs and overwritten keychords show up properly in the SUI

commit 12a61c5
Author: Pankaj Bhojwani <[email protected]>
Date:   Thu Apr 25 19:16:27 2024 -0700

    shows up in sui and all keybindings work

commit d0938e2
Author: Pankaj Bhojwani <[email protected]>
Date:   Wed Apr 24 18:15:37 2024 -0700

    ugly way to make sure we fixup

commit f425746
Author: Pankaj Bhojwani <[email protected]>
Date:   Wed Apr 24 16:21:22 2024 -0700

    remove keysmap

commit e28d478
Author: Pankaj Bhojwani <[email protected]>
Date:   Wed Apr 24 16:04:39 2024 -0700

    some todos for later

commit 0a3e17e
Author: Pankaj Bhojwani <[email protected]>
Date:   Wed Apr 24 15:33:35 2024 -0700

    edge cases

commit 22ab936
Author: Pankaj Bhojwani <[email protected]>
Date:   Wed Apr 24 14:33:01 2024 -0700

    works??

commit c134402
Author: Pankaj Bhojwani <[email protected]>
Date:   Wed Apr 24 11:24:40 2024 -0700

    about to test stage 1

commit 85933e2
Author: Pankaj Bhojwani <[email protected]>
Date:   Tue Apr 23 09:49:27 2024 -0700

    midpoint

commit ca3eb87
Author: Pankaj Bhojwani <[email protected]>
Date:   Wed Apr 17 16:58:04 2024 -0700

    rename and comment

commit 5e70911
Author: Pankaj Bhojwani <[email protected]>
Date:   Wed Apr 17 16:49:58 2024 -0700

    remove 0

commit 360b92e
Author: Pankaj Bhojwani <[email protected]>
Date:   Wed Apr 17 16:38:52 2024 -0700

    fmt_compile, fix test

commit 5ee630e
Author: Pankaj Bhojwani <[email protected]>
Date:   Fri Apr 12 15:16:36 2024 -0700

    fmt is smart

commit aa49212
Author: Pankaj Bhojwani <[email protected]>
Date:   Fri Apr 12 15:08:53 2024 -0700

    null check

commit 12f3aa9
Author: Pankaj Bhojwani <[email protected]>
Date:   Fri Apr 12 15:04:23 2024 -0700

    truncate and hex, debug assert

commit bdf42c2
Author: Pankaj Bhojwani <[email protected]>
Date:   Thu Apr 11 16:05:35 2024 -0700

    first round of nits

commit af2d22f
Merge: 6e293a5 5f3a857
Author: Pankaj Bhojwani <[email protected]>
Date:   Thu Apr 11 09:42:42 2024 -0700

    defaults conflict

commit 6e293a5
Author: Pankaj Bhojwani <[email protected]>
Date:   Mon Apr 1 10:26:54 2024 -0700

    Everytime

commit dd25ed7
Author: Pankaj Bhojwani <[email protected]>
Date:   Mon Apr 1 10:23:23 2024 -0700

    change tests

commit dca7df5
Author: Pankaj Bhojwani <[email protected]>
Date:   Fri Mar 29 13:49:33 2024 -0700

    excess line

commit 9fc6972
Author: Pankaj Bhojwani <[email protected]>
Date:   Fri Mar 29 13:40:53 2024 -0700

    add tests

commit 5c2307c
Author: Pankaj Bhojwani <[email protected]>
Date:   Thu Mar 28 11:48:51 2024 -0700

    fix test

commit d57c7a1
Author: Pankaj Bhojwani <[email protected]>
Date:   Wed Mar 27 18:04:12 2024 -0700

    move this

commit 71bf90f
Author: Pankaj Bhojwani <[email protected]>
Date:   Wed Mar 27 18:02:17 2024 -0700

    even better, also get the ID from json

commit 10d1fc8
Author: Pankaj Bhojwani <[email protected]>
Date:   Wed Mar 27 17:30:55 2024 -0700

    this way is better

commit 44510dc
Author: Pankaj Bhojwani <[email protected]>
Date:   Wed Mar 27 17:14:21 2024 -0700

    move id generation to fixupusersettings

commit eccd87f
Author: Pankaj Bhojwani <[email protected]>
Date:   Wed Mar 27 15:38:50 2024 -0700

    update comment

commit 6c32539
Author: Pankaj Bhojwani <[email protected]>
Date:   Wed Mar 27 15:37:19 2024 -0700

    string of numbers is unsightly but it works

commit 2093660
Author: Pankaj Bhojwani <[email protected]>
Date:   Tue Mar 26 11:45:44 2024 -0700

    line

commit b43191d
Author: Pankaj Bhojwani <[email protected]>
Date:   Tue Mar 26 11:44:47 2024 -0700

    spacing

commit 7c907fe
Merge: db528c9 e8f18ea
Author: Pankaj Bhojwani <[email protected]>
Date:   Tue Mar 26 11:43:50 2024 -0700

    nits

commit db528c9
Author: Pankaj Bhojwani <[email protected]>
Date:   Tue Mar 26 11:29:12 2024 -0700

    generate IDs for user commands

commit be193b2
Merge: 2bb1b6c 5f27280
Author: Pankaj Bhojwani <[email protected]>
Date:   Tue Mar 19 15:08:08 2024 -0700

    merge origin

commit 2bb1b6c
Merge: 66fe08f 5383cb3
Author: Pankaj Bhojwani <[email protected]>
Date:   Tue Mar 19 12:18:06 2024 -0700

    conflict

commit 66fe08f
Author: Pankaj Bhojwani <[email protected]>
Date:   Wed Mar 6 17:24:39 2024 -0800

    default ids

commit 642d0ab
Author: Pankaj Bhojwani <[email protected]>
Date:   Tue Mar 5 15:47:49 2024 -0800

    inbox makes more sense

commit 8cc82de
Author: Pankaj Bhojwani <[email protected]>
Date:   Tue Mar 5 15:42:38 2024 -0800

    generated

commit 052d063
Author: Pankaj Bhojwani <[email protected]>
Date:   Tue Mar 5 14:36:45 2024 -0800

    ah one of the tests uses this

commit 8bcbd0b
Author: Pankaj Bhojwani <[email protected]>
Date:   Tue Mar 5 14:20:14 2024 -0800

    fix tests

commit 9dff28f
Author: Pankaj Bhojwani <[email protected]>
Date:   Tue Mar 5 13:51:18 2024 -0800

    update calls in tests

commit 90627b3
Author: Pankaj Bhojwani <[email protected]>
Date:   Tue Mar 5 11:35:26 2024 -0800

    add origin tag
.wt.json Fixed Show fixed Hide fixed

This comment has been minimized.

.wt.json Outdated Show resolved Hide resolved
src/cascadia/TerminalSettingsModel/CascadiaSettings.cpp Outdated Show resolved Hide resolved
src/cascadia/TerminalSettingsModel/CascadiaSettings.cpp Outdated Show resolved Hide resolved
src/cascadia/TerminalSettingsModel/CascadiaSettings.cpp Outdated Show resolved Hide resolved
src/cascadia/TerminalSettingsModel/ActionMap.cpp Outdated Show resolved Hide resolved
src/cascadia/TerminalSettingsModel/ActionMap.cpp Outdated Show resolved Hide resolved
src/cascadia/TerminalSettingsModel/ActionMap.cpp Outdated Show resolved Hide resolved
src/cascadia/TerminalSettingsModel/ActionMap.cpp Outdated Show resolved Hide resolved
winrt::Windows::Foundation::IAsyncOperation<IVector<Model::Command>> ActionMap::FilterToSnippets(
winrt::hstring currentCommandline,
winrt::hstring currentWorkingDirectory)
{
Copy link
Member

Choose a reason for hiding this comment

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

Do we want to assert or switch over to a thread here?

Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't think we needed to. _updateLocalSnippetCache would move to a bg thread if it needs to (we may not even need to call it). And the whole FilterToSnippets is an IAsyncOperation so the caller should know it's not necessarily returning on the same thread.

@zadjii-msft zadjii-msft added this to the Terminal v1.22 milestone Jul 2, 2024
@zadjii-msft
Copy link
Member Author

Added the til::io reader and fixed the locking on the CWD action cache

src/cascadia/TerminalSettingsModel/ActionMap.cpp Outdated Show resolved Hide resolved
Comment on lines +927 to +930
if (!localTasksFileContents.has_value() || localTasksFileContents->empty())
{
return {};
}
Copy link
Member

Choose a reason for hiding this comment

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

ARGH Dustin my review comment about the optionals! 😄 I'm gonna fix that.

Copy link
Member

Choose a reason for hiding this comment

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

haha thanks

Copy link
Member

@carlos-zamora carlos-zamora left a comment

Choose a reason for hiding this comment

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

Just a few nits, but nothing worth blocking over

src/cascadia/TerminalSettingsModel/Command.cpp Outdated Show resolved Hide resolved
src/cascadia/TerminalSettingsModel/ActionMap.cpp Outdated Show resolved Hide resolved
@zadjii-msft zadjii-msft merged commit 21fa303 into main Jul 26, 2024
21 of 25 checks passed
@zadjii-msft zadjii-msft deleted the dev/migrie/f/local-snippets-final-FINAL branch July 26, 2024 01:39
@carlos-zamora carlos-zamora removed their assignment Jul 30, 2024
Comment on lines +927 to +930
if (!localTasksFileContents.has_value() || localTasksFileContents->empty())
{
return {};
}
Copy link
Member

Choose a reason for hiding this comment

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

haha thanks

// .wt.json in this directory. If it exists, we'll read it, parse it's JSON,
// then take all the sendInput actions in it and store them in our
// _cwdLocalSnippetsCache
std::vector<Model::Command> ActionMap::_updateLocalSnippetCache(winrt::hstring currentWorkingDirectory)
Copy link
Member

Choose a reason for hiding this comment

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

I'm actually fairly uncomfortable with this behavior living in SettingsModel, but oh well

Copy link
Member

Choose a reason for hiding this comment

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

(it's where the json parser lives, so it makes sense)

std::vector<Command> commandsCollection;
Control::CommandHistoryContext context{ nullptr };
winrt::hstring currentCommandline = L"";
winrt::hstring currentWorkingDirectory = L"";
Copy link
Member

Choose a reason for hiding this comment

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

nit: always prefer {} for hstrings. no need to have empty string literals all over the place.

Copy link
Member

Choose a reason for hiding this comment

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

I've previously raised a nit about how I personally think that {} is perfect for object constructions, while = is perfect for assignments, in particular of trivial types. I know you all like to write auto foo{ bar() }, so I feel quite betrayed seeing this!
(This is not a serious comment. 😄)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants