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.

Sign up for GitHub

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

Jump to bottom

Spec for global action IDs #7175

Merged
merged 1 commit into from
Aug 12, 2020
Merged

Spec for global action IDs #7175

merged 1 commit into from
Aug 12, 2020

Conversation

zadjii-msft
Copy link
Member

Summary of the Pull Request

⚠️ This spec has been moved from #6902. That version was branched off the new tab menu customization, and had a terribly convoluted git history. After discussion with the team, we've decided that it's best that this spec is merged atomically first, and used as the basis for #5888, as opposed to the other way around.

This document is intended to serve as an addition to the Command Palette Spec,
as well as the New Tab Menu Customization Spec.

As we come to rely more on actions being a mechanism by which the user defines
"do something in the Terminal", we'll want to make it even easier for users to
re-use the actions that they've already defined, as to reduce duplicated json as
much as possible. This spec proposes a mechanism by which actions could be
uniquely identifiable, so that the user could refer to bindings in other
contexts without needing to replicate an entire json blob.

PR Checklist

Detailed Description of the Pull Request / Additional comments

*** read the spec ***

@zadjii-msft zadjii-msft added Issue-Docs It's a documentation issue that really should be on MicrosoftDocs/Console-Docs Area-Settings Issues related to settings and customizability, for console or terminal Product-Terminal The new Windows Terminal. labels Aug 4, 2020
@zadjii-msft zadjii-msft mentioned this pull request Aug 4, 2020
Closed
3 tasks
carlos-zamora
carlos-zamora approved these changes Aug 4, 2020
View reviewed changes
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.

Looks great! :)

Urgh. This is going to conflict with #7141 (specifically the keybindings part). I think I'll definitely have to make WinRT-ify-ing the keybindings its own PR. I'm sure I'll be reaching out to you for help though so we'll have an idea of what's coming.

leonMSFT
leonMSFT approved these changes Aug 10, 2020
View reviewed changes
Copy link
Contributor

@leonMSFT leonMSFT left a comment •
edited
Loading

Choose a reason for hiding this comment

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

This makes a lot of sense! I really like the whole ID idea and being able to reference it elsewhere in settings.

doc/specs/#6899 - Action IDs/#6899 - Action IDs.md Show resolved Hide resolved
DHowett
DHowett approved these changes Aug 12, 2020
View reviewed changes
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.

SHIP IT

doc/specs/#6899 - Action IDs/#6899 - Action IDs.md Show resolved Hide resolved
doc/specs/#6899 - Action IDs/#6899 - Action IDs.md Show resolved Hide resolved
@DHowett DHowett removed their assignment Aug 12, 2020
@zadjii-msft zadjii-msft merged commit c241f83 into master Aug 12, 2020
@zadjii-msft zadjii-msft deleted the dev/migrie/s/action-ids-v2 branch August 12, 2020 13:39
@zadjii-msft zadjii-msft mentioned this pull request Aug 18, 2020
Merged
ghost pushed a commit that referenced this pull request Aug 20, 2020
@zadjii-msft
Replace "bindings" with "actions" (#7332)
2c4b868 
In #6532, we thought it would be a good idea to add "bindings" as an
overload for "keybindings", as we were no longer going to use the
keybindings array for just keybindings. We were going to add commands.
So we started secretly treating `"bindings"` the same as
`"keybindings"`.

Then, in #7175, we discussed using "actions" as the key for the list of
commands/keybindings/global actions, instead of using "bindings". We're
going to be using this array as the global list of all actions, so it
makes sense to just call it `"actions"`. 

This PR renames "bindings" to "actions". Fortunately, we never
documented the "bindings" overload in the first place, so we can get
away with this safely, and preferably before we ship "bindings" for too
long.

References #6899
DHowett pushed a commit that referenced this pull request Aug 20, 2020
@zadjii-msft @DHowett
Replace "bindings" with "actions" (#7332)
33a1a11 
In #6532, we thought it would be a good idea to add "bindings" as an
overload for "keybindings", as we were no longer going to use the
keybindings array for just keybindings. We were going to add commands.
So we started secretly treating `"bindings"` the same as
`"keybindings"`.

Then, in #7175, we discussed using "actions" as the key for the list of
commands/keybindings/global actions, instead of using "bindings". We're
going to be using this array as the global list of all actions, so it
makes sense to just call it `"actions"`.

This PR renames "bindings" to "actions". Fortunately, we never
documented the "bindings" overload in the first place, so we can get
away with this safely, and preferably before we ship "bindings" for too
long.

References #6899

(cherry picked from commit 2c4b868)
MichelleTanPY pushed a commit to MichelleTanPY/terminal that referenced this pull request Aug 20, 2020
@zadjii-msft @MichelleTanPY
Replace "bindings" with "actions" (microsoft#7332)
d42a783 
In microsoft#6532, we thought it would be a good idea to add "bindings" as an
overload for "keybindings", as we were no longer going to use the
keybindings array for just keybindings. We were going to add commands.
So we started secretly treating `"bindings"` the same as
`"keybindings"`.

Then, in microsoft#7175, we discussed using "actions" as the key for the list of
commands/keybindings/global actions, instead of using "bindings". We're
going to be using this array as the global list of all actions, so it
makes sense to just call it `"actions"`. 

This PR renames "bindings" to "actions". Fortunately, we never
documented the "bindings" overload in the first place, so we can get
away with this safely, and preferably before we ship "bindings" for too
long.

References microsoft#6899
@zadjii-msft zadjii-msft mentioned this pull request Oct 19, 2020
Merged
3 tasks
@carlos-zamora carlos-zamora mentioned this pull request Oct 29, 2020
Open
@zadjii-msft zadjii-msft mentioned this pull request Jan 5, 2021
Closed
@carlos-zamora carlos-zamora mentioned this pull request Feb 11, 2021
Open
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@leonMSFT leonMSFT leonMSFT approved these changes

@carlos-zamora carlos-zamora carlos-zamora approved these changes

@DHowett DHowett DHowett approved these changes

Assignees

@miniksa miniksa

@cinnamon-msft cinnamon-msft

Labels
Area-Settings Issues related to settings and customizability, for console or terminal Issue-Docs It's a documentation issue that really should be on MicrosoftDocs/Console-Docs Product-Terminal The new Windows Terminal.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@zadjii-msft @DHowett @carlos-zamora @leonMSFT @miniksa @cinnamon-msft