Hide "Open in Terminal" context menu option appropriately

[leejy12]

This commit hides the "Open in Terminal" context menu option when the
context menu is opened in a non-filesystem path like "Quick Actions".

Closes #12578

[github-actions]

@check-spelling-bot Report

Unrecognized words, please review:

• SFGAO
• SFGAOF

Previously acknowledged words that are now absent

azurewebsites Checkin condev Consolescreen css cxcy DCompile debolden deconstructed devicefamily dxp errno FARPROC GETKEYSTATE guardxfg HPAINTBUFFER HPROPSHEETPAGE iconify ipa LLVM LPCHARSETINFO MAPVIRTUALKEY MSDL mspartners ned newcursor nlength NOWAIT PENDTASKMSG pgorepro pgort PGU Poli PPORT redistributable SOURCESDIRECTORY Timeline timelines toolbars unintense UWA UWAs VKKEYSCAN wddmconrenderer wdx windev WResult xfg

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 git@github.com:leejy12/terminal.git repository
on the main branch:

update_files() {
perl -e '
my @expect_files=qw('".github/actions/spelling/expect/alphabet.txt
.github/actions/spelling/expect/expect.txt
.github/actions/spelling/expect/web.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/spelling/expect/11b810e4036b669e5f223cce447f76d56044be0f.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);
'
}

comment_json=$(mktemp)
curl -L -s -S \
  --header "Content-Type: application/json" \
  "https://api.github.com/repos/microsoft/terminal/issues/comments/1141017169" > "$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")
  
update_files
rm $comment_body
git add -u

✏️ Contributor please read this

By default the command suggestion will generate a file named based on your commit. That's generally ok as long as you add the file to your commit. Someone can reorganize it later.

⚠️ The command is written for posix shells. You can copy the contents of each perl command excluding the outer ' marks and dropping any '"/"' quotation mark pairs into a file and then run perl file.pl from the root of the repository to run the code. Alternatively, you can manually insert the items...

If the listed items are:

• ... misspelled, then please correct them instead of using the command.
• ... names, please add them to .github/actions/spelling/allow/names.txt.
• ... APIs, you can add them to a file in .github/actions/spelling/allow/.
• ... just things you're using, please add them to an appropriate file in .github/actions/spelling/expect/.
• ... tokens you only need in one place and shouldn't generally be used, you can add an item in an appropriate file in .github/actions/spelling/patterns/.

See the README.md in each directory for more information.

🔬 You can test your commits without appending to a PR by creating a new branch with that extra change and pushing it to your fork. The check-spelling action will run in response to your push -- it doesn't require an open pull request. By using such a branch, you can limit the number of typos your peers see you make. 😉

🗜️ 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 a patterns/{file}.txt.

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).

[leejy12]

Before & After

[lhecker]

Can psiItemArray be nullptr? If it's usually never nullptr, we should probably just return ECS_ENABLED. This would at least not be worse than the previous solution.

And I believe _GetPathFromExplorer might be too slow for fOkToBeSlow == FALSE. 🤔

[leejy12]

It seems that if we open the context menu on a blank area of Explorer (no items selected), then psiItemArray is nullptr.

https://streamable.com/p7a1fw

[DHowett]

@lhecker there's also an issue in older versions of Windows where psiItemArray would be nullptr even when the user had selected an item.

[lhecker]

FYI I've slightly edited your pull request message to include a brief description and to remove the <!-- ... --> markers. 🙂

[lhecker]

We might want to consider checking fOkToBeSlow before doing _GetPathFromExplorer. 🤔
(After measuring how long _GetPathFromExplorer needs on average.)

[leejy12]

I just checked locally, and fOkToBeSlow is TRUE, so I think it's ok to leave it this way.

On my PC, _GetPathFromExplorer takes no longer than 50ms, so the performance impact is not very apparent.

I think on less performant devices, the context menu could flicker for a split second because the verb is initially "Loading..." in gray, which eventually gets replaced with "Open in Terminal" or gets hidden.

[carlos-zamora]

Great! Thank you!

[carlos-zamora]

@msftbot merge this in 5 minutes

[ghost]

Hello @carlos-zamora!

Because you've given me some instructions on how to help merge this pull request, I'll be modifying my merge approach. Here's how I understand your requirements for merging this pull request:

• I won't merge this pull request until after the UTC date Tue, 31 May 2022 17:46:49 GMT, which is in 5 minutes

If this doesn't seem right to you, you can tell me to cancel these instructions and use the auto-merge policy that has been configured for this repository. Try telling me "forget everything I just told you".

[DHowett]

Thanks for fixing this @leejy12!

[ghost]

🎉Windows Terminal v1.14.186 has been released which incorporates this pull request.:tada:

Handy links:

• Release Notes
• Store Download

[ghost]

🎉Windows Terminal Preview v1.15.186 has been released which incorporates this pull request.:tada:

Handy links:

• Release Notes
• Store Download

[zadjii-msft]

Okay I'm pretty sure this caused #13523. Turns out that there's a non-zero rate of the shell giving us 0 items in the array. I can check for that easy enough, but it seems like still returning ECS_HIDDEN for that results in the menu entry not always appearing for the desktop.

I'll need to noodle more

[zadjii-msft]

Yea, unless I get something more clever to actually re-enable this, then I think we're going to have to revert this. I suspect that having it hidden on the desktop is just more painful than having it visible in "My PC". I've got a line out to try and learn more about why there might be 0 entries, but unless we come up with something fast, I think we should revert this.

[DHowett]

We used to always return ECS_ENABLED. Could we sanely default to enabled if there are 0 items? That lets us determine when to show ourselves if we can determine the type, and avoid breaking when we can't but were somehow queried anyway (Desktop case)

[zadjii-msft]

Alas, it seems that the "This PC" case AND the Desktop BG case both hit the null array path from my limited testing. I wish there was a better way to debug attach to the dllhost that spawns for this 😕

[zadjii-msft]

see also https://github.com/microsoft/terminal/pull/new/dev/migrie/b/13523-context-menu

[ghost]

🎉Windows Terminal v1.14.228 has been released which incorporates this pull request.:tada:

Handy links:

• Release Notes
• Store Download

[ghost]

🎉Windows Terminal Preview v1.15.228 has been released which incorporates this pull request.:tada:

Handy links:

• Release Notes
• Store Download