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

[PT Run] Windows Terminal Plugin #13367

Merged
merged 2 commits into from
Oct 4, 2021

Conversation

davidegiacometti
Copy link
Collaborator

@davidegiacometti davidegiacometti commented Sep 22, 2021

Summary of the Pull Request

What is this about:
Open in PT Run Windows Terminal profiles

Screenshot 2021-10-02 141952

Screenshot 2021-10-02 141556

TO-DO

  • Setup
  • Enable localization
  • Plugin icon
  • Plugin results icon
  • Test on Windows 11
  • Add tests

What is include in the PR:

  • Support both Windows Terminal and Windows Terminal Preview
  • The plugin read the profiles from the settings JSON file
  • Option for open the profiles in a new tab

How does someone test / validate:

  • Validate that the new plugin is working as expected

Quality Checklist

Contributor License Agreement (CLA)

A CLA must be signed. If not, go over here and sign the CLA.

@davidegiacometti davidegiacometti added Product-PowerToys Run Improved app launch PT Run (Win+R) Window Run-Plugin Things that relate with PowerToys Run's plugin interface labels Sep 22, 2021
@davidegiacometti davidegiacometti marked this pull request as draft September 22, 2021 20:24
@github-actions
Copy link

github-actions bot commented Sep 22, 2021

@check-spelling-bot Report

Unrecognized words, please review:

  • ITerminal
Previously acknowledged words that are now absent Accessible available chrisharris coc CTriage djsoref docsmsft dogancelik estdir Fody ftps gmx htt inprivate installpowertoys itsme listbox mfreadwrite mfuuid Nefario nitroin null nunit powertoyswiki Radiobuttons sidepanel spamming systray ulazy windevbuildagents winstore xia XSmall xunit
Some files were were automatically ignored

These sample patterns would exclude them:

^src/modules/previewpane/UnitTests-MarkdownPreviewHandler/HelperFiles/MarkdownWithHTMLImageTag\.txt$

You should consider adding them to:

.github/actions/spell-check/excludes.txt

File matching is via Perl regular expressions.

To check these files, more of their words need to be in the dictionary than not. You can use patterns.txt to exclude portions, add items to the dictionary (e.g. by adding them to allow.txt), or fix typos.

To accept these unrecognized words as correct (and remove the previously acknowledged and now absent words), run the following commands

... in a clone of the [email protected]:microsoft/PowerToys.git repository
on the users/davidegiacometti/terminal-plugin branch:

update_files() {
perl -e '
my @expect_files=qw('".github/actions/spell-check/expect.txt"');
@ARGV=@expect_files;
my @stale=qw('"$patch_remove"');
my $re=join "|", @stale;
my $suffix=".".time();
my $previous="";
sub maybe_unlink { unlink($_[0]) if $_[0]; }
while (<>) {
if ($ARGV ne $old_argv) { maybe_unlink($previous); $previous="$ARGV$suffix"; rename($ARGV, $previous); open(ARGV_OUT, ">$ARGV"); select(ARGV_OUT); $old_argv = $ARGV; }
next if /^(?:$re)(?:(?:\r|\n)*$| .*)/; print;
}; maybe_unlink($previous);'
perl -e '
my $new_expect_file=".github/actions/spell-check/expect.txt";
use File::Path qw(make_path);
use File::Basename qw(dirname);
make_path (dirname($new_expect_file));
open FILE, q{<}, $new_expect_file; chomp(my @words = <FILE>); close FILE;
my @add=qw('"$patch_add"');
my %items; @items{@words} = @words x (1); @items{@add} = @add x (1);
@words = sort {lc($a)."-".$a cmp lc($b)."-".$b} keys %items;
open FILE, q{>}, $new_expect_file; for my $word (@words) { print FILE "$word\n" if $word =~ /\w/; };
close FILE;
system("git", "add", $new_expect_file);
'
(cat '.github/actions/spell-check/excludes.txt' - <<EOF
$should_exclude_patterns
EOF
) |grep .|
sort -f |
uniq > '.github/actions/spell-check/excludes.txt.temp' &&
mv '.github/actions/spell-check/excludes.txt.temp' '.github/actions/spell-check/excludes.txt'
}

comment_json=$(mktemp)
curl -L -s -S \
  --header "Content-Type: application/json" \
  "https://api.github.com/repos/microsoft/PowerToys/issues/comments/925304808" > "$comment_json"
comment_body=$(mktemp)
jq -r .body < "$comment_json" > $comment_body
rm $comment_json

patch_remove=$(perl -ne 'next unless s{^</summary>(.*)</details>$}{$1}; print' < "$comment_body")
  

patch_add=$(perl -e '$/=undef;
$_=<>;
s{<details>.*}{}s;
s{^#.*}{};
s{\n##.*}{};
s{(?:^|\n)\s*\*}{}g;
s{\s+}{ }g;
print' < "$comment_body")
  

should_exclude_patterns=$(perl -e '$/=undef;
$_=<>;
exit unless s{(?:You should consider excluding directory paths|You should consider adding them to).*}{}s;
s{.*These sample patterns would exclude them:}{}s;
s{.*\`\`\`([^`]*)\`\`\`.*}{$1}m;
print' < "$comment_body" | grep . || true)

update_files
rm $comment_body
git add -u
If you see a bunch of garbage

If it relates to a ...

well-formed pattern

See if there's a pattern that would match it.

If not, try writing one and adding it to the patterns.txt file.

Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines.

Note that patterns can't match multiline strings.

binary-ish string

Please add a file path to the excludes.txt file instead of just accepting the garbage.

File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.

^ refers to the file's path from the root of the repository, so ^README\.md$ would exclude README.md (on whichever branch you're using).

@htcfreek
Copy link
Collaborator

Does it make sense to show the results when searching for wt/Windows Terminal (Preview) without using activation command (global results).

@niels9001
Copy link
Contributor

niels9001 commented Sep 23, 2021

@davidegiacometti I have created a .light and .dark icon that might be useful for this plugin :) (and already in the style for #13385)

terminal light
terminal dark

@Aaron-Junker
Copy link
Collaborator

@davidegiacometti I have created a .light and .dark icon that might be useful for this plugin :) (and already in the style for #13385)

terminal light
terminal dark

@niels9001 I like these icons, but in my opinion it would be better to just use the terminal icon (or at least in the results):

image

@davidegiacometti
Copy link
Collaborator Author

@niels9001 icon must be used for the settings.
For results I would like to use the profile icons from Terminal but the icon could be an image file or a resource inside Terminal package.
Need to look in to this. Otherwise let's consider plugin or Terminal icon.

@Jay-o-Way
Copy link
Collaborator

I have created a .light and .dark icon

Why not stick to MDL2?

@davidegiacometti
Copy link
Collaborator Author

I have created a .light and .dark icon

Why not stick to MDL2?

Icons for plugins aren't MDL2 but custom

@davidegiacometti davidegiacometti force-pushed the users/davidegiacometti/terminal-plugin branch from 4c1aacb to b326651 Compare September 26, 2021 08:16
@github-actions

This comment has been minimized.

@davidegiacometti
Copy link
Collaborator Author

Post about result's icon: #4194 (comment)

@niels9001
Copy link
Contributor

Post about result's icon: #4194 (comment)

I'd go for the colored Terminal icon in Run itself, but the Fluent Icons-version for the plugin manager list in Settings.

@github-actions

This comment has been minimized.

@davidegiacometti davidegiacometti force-pushed the users/davidegiacometti/terminal-plugin branch from edb2a6a to e12a8fd Compare October 2, 2021 10:32
@github-actions

This comment has been minimized.

@davidegiacometti davidegiacometti force-pushed the users/davidegiacometti/terminal-plugin branch from e12a8fd to 2050604 Compare October 2, 2021 11:03
@davidegiacometti davidegiacometti marked this pull request as ready for review October 2, 2021 12:20
@davidegiacometti
Copy link
Collaborator Author

The PR is ready to review.
I have updated the screenshots in the first post.

Somebody please can build and test the plugin on Windows 11?

@jaimecbernardo
Copy link
Collaborator

Works nicely on Windows 11.

Copy link
Collaborator

@jaimecbernardo jaimecbernardo left a comment

Choose a reason for hiding this comment

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

LGTM! Works well.
I'd still like to have another review in here, but after that it can go in.
Thank you for the contribution!

Copy link
Collaborator

@yuyoyuppe yuyoyuppe left a comment

Choose a reason for hiding this comment

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

The code looks good to me, awesome contribution!

@davidegiacometti davidegiacometti deleted the users/davidegiacometti/terminal-plugin branch October 7, 2021 18:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Product-PowerToys Run Improved app launch PT Run (Win+R) Window Run-Plugin Things that relate with PowerToys Run's plugin interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants