feat: Add DENO_NO_PROMPT variable #14209
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@ry please review. Nayeem's rationale from the issue makes sense, but I don't have strong opinion either way |
bartlomieju
reviewed
Apr 10, 2022
There was a problem hiding this comment.
Does DENO_NO_PROMPT=0 cause it to be enabled or disabled?
LGTM though, I am in favour of this feature.
@ry PTAL
bartlomieju
reviewed
Apr 13, 2022
ry
reviewed
Apr 13, 2022
|
LGTM - but the two comments should be addressed. |
|
Please open a PR to https://github.com/denoland/manual and add this env variable to "Setup your environment chapter" |
Enabled same as |
That's inconsistent with |
|
Ready for review |
rivy
added a commit
to rivy/js.os-paths
that referenced
this pull request
Aug 9, 2022
- revert to earlier deno::std library version without NODE_DEBUG permission prompts - wip why/refs # [why] In the change from deno::std@0.134.0 to deno::std@0.135.0, a change was made to remove a top-level async permission query gate from the library, replacing it with a synchronous `try...catch`. This was done to avoid unexpected module load order since the module with `await` will run at a time later than synchronous peers (although still prior to user code). Unfortunately, the now-default behavior of Deno is to prompt on use for permissions which aren't 'granted' or 'denied'. This leads to ugly UI/UX for scripts which try to control their logging and UI (as well as being confusing and unexpected for non-developer users). This UI/UX behavior is only avoidable if the script is run with the `--no-prompt` option flag or all needed permissions enabled, encouraging use of `--no-prompt` and, more problematically, `--allow-all`. As of 2022-08, with the current Deno (Deno v1.8.0+) and deno::std (deno::std@0.135.0+) library implementation, no work-around is possible due to: 1. being impossible to actually revoke a permission (`revoke` just "downgrade[s] a permission from 'granted' to 'prompt'); 2. the permission API has no synchronous functions making revocation impossible prior to a synchronous access (even if a polyfill patch via module was attempted). Reversion and pinning of deno::std to v0.134.0 avoids the patch changing from async gating to sync `try...catch` while still maintaining most of the functionality of later deno::std library versions. # refs [DENO_NO_PROMPT env var support](denoland/deno#14208) [feat: Add DENO_NO_PROMPT variable](denoland/deno#14209) [discussion: Bring back permission prompt behind a flag](denoland/deno#3811) [Security prompt by default (instead of throw)](denoland/deno#10183) [Bad UX with prompt by default](denoland/deno#13730) [`deno repl` has permissions by default?](denoland/deno#12665) [permission prompt problems](denoland/deno#11936) [Design Meeting 2021-07-29 ~ `Prompt by default`](denoland/deno#11767) [Seeking a better UX for permissions](denoland/deno#11061) [HowTO test that a module is *no-panic* and *no-prompt* when statically imported?](denoland/deno#15356)
rivy
added a commit
to rivy/js.os-paths
that referenced
this pull request
Aug 9, 2022
…mpt(s) - revert to earlier deno::std library version without NODE_DEBUG permission prompts # [why] In the change from deno::std@0.134.0 to deno::std@0.135.0, a change was made to remove a top-level async permission query gate from the library, replacing it with a synchronous `try...catch`.[1] This was done to avoid unexpected module load order since the module with `await` will run at a time later than synchronous peers (although still prior to user code).[2,3] Unfortunately, the, now default, behavior of Deno is to prompt on use for permissions which aren't 'granted' or 'denied'. This leads to ugly UI/UX for scripts which try to control their logging and UI (as well as being confusing and unexpected for non-developer users). This UI/UX behavior is only avoidable if the script is run with the `--no-prompt` option flag or all needed permissions enabled, encouraging use of `--no-prompt` and, more problematically, `--allow-all`. As of 2022-08, with the current Deno (Deno v1.8.0+) and deno::std (deno::std@0.135.0+) library implementation, no work-around is possible due to: 1. being impossible to actually revoke a permission (`revoke` just "downgrade[s] a permission from 'granted' to 'prompt'); 2. the permission API has no synchronous functions making revocation impossible prior to a synchronous access (even if a polyfill patch via module was attempted). Reversion and pinning of deno::std to v0.134.0 avoids the patch changing from async gating to sync `try...catch` while still maintaining most of the functionality of later deno::std library versions. # refs [1] [fix(node): Make global.ts evaluate synchronously](denoland/std#2098) [2] [Execution order of imports in deno is unclear/unexpected](denoland/deno#14243) [3] [std/node should avoid TLA](denoland/std#2097) ## related discussion/issues [Discussion ~ Bring back permission prompt behind a flag](denoland/deno#3811) [Security prompt by default (instead of throw)](denoland/deno#10183) [Seeking a better UX for permissions](denoland/deno#11061) [Design Meeting 2021-07-29 ~ `Prompt by default`](denoland/deno#11767) [permission prompt problems](denoland/deno#11936) [`deno repl` has permissions by default?](denoland/deno#12665) [Bad UX with prompt by default](denoland/deno#13730) [DENO_NO_PROMPT env var support](denoland/deno#14208) [feat: Add DENO_NO_PROMPT variable](denoland/deno#14209) [HowTO test that a module is *no-panic* and *no-prompt* when statically imported?](denoland/deno#15356)
rivy
added a commit
to rivy/js.os-paths
that referenced
this pull request
Aug 9, 2022
…mpt(s) - revert to earlier deno::std library version without NODE_DEBUG permission prompts # [why] In the change from deno::std@0.134.0 to deno::std@0.135.0, a change was made to remove a top-level async permission query gate from the library, replacing it with a synchronous `try...catch`.[1] This was done to avoid unexpected module load order since the module with `await` will run at a time later than synchronous peers (although still prior to user code).[2,3] Unfortunately, the, now default, behavior of Deno is to prompt on use for permissions which aren't 'granted' or 'denied'. This leads to ugly UI/UX for scripts which try to control their logging and UI (as well as being confusing and unexpected for non-developer users). This UI/UX behavior is only avoidable if the script is run with the `--no-prompt` option flag or all needed permissions enabled, encouraging use of `--no-prompt` and, more problematically, `--allow-all`. As of 2022-08, with the current Deno (Deno v1.8.0+) and deno::std (deno::std@0.135.0+) library implementation, no work-around is possible due to: 1. being impossible to actually revoke a permission (`revoke` just "downgrade[s] a permission from 'granted' to 'prompt'); 2. the permission API has no synchronous functions making revocation impossible prior to a synchronous access (even if a polyfill patch via module was attempted). Reversion and pinning of deno::std to v0.134.0 avoids the patch changing from async gating to sync `try...catch` while still maintaining most of the functionality of later deno::std library versions. # refs [1] [fix(node): Make global.ts evaluate synchronously](denoland/std#2098) [2] [Execution order of imports in deno is unclear/unexpected](denoland/deno#14243) [3] [std/node should avoid TLA](denoland/std#2097) ## related discussion/issues [Discussion ~ Bring back permission prompt behind a flag](denoland/deno#3811) [Security prompt by default (instead of throw)](denoland/deno#10183) [Seeking a better UX for permissions](denoland/deno#11061) [Design Meeting 2021-07-29 ~ `Prompt by default`](denoland/deno#11767) [permission prompt problems](denoland/deno#11936) [`deno repl` has permissions by default?](denoland/deno#12665) [Bad UX with prompt by default](denoland/deno#13730) [DENO_NO_PROMPT env var support](denoland/deno#14208) [feat: Add DENO_NO_PROMPT variable](denoland/deno#14209) [HowTO test that a module is *no-panic* and *no-prompt* when statically imported?](denoland/deno#15356)
rivy
added a commit
to rivy/js.os-paths
that referenced
this pull request
Aug 9, 2022
…mpt(s) - revert to earlier deno::std library version without NODE_DEBUG permission prompts # [why] In the change from deno::std@0.134.0 to deno::std@0.135.0, a change was made to remove a top-level async permission query gate from the library, replacing it with a synchronous `try...catch`.[1] This was done to avoid unexpected module load order since the module with `await` will run at a time later than synchronous peers (although still prior to user code).[2,3] Unfortunately, the, now default, behavior of Deno is to prompt on use for permissions which aren't 'granted' or 'denied'. This leads to ugly UI/UX for scripts which try to control their logging and UI (as well as being confusing and unexpected for non-developer users). This UI/UX behavior is only avoidable if the script is run with the `--no-prompt` option flag or all needed permissions enabled, encouraging use of `--no-prompt` and, more problematically, `--allow-all`. As of 2022-08, with the current Deno (Deno v1.8.0+) and deno::std (deno::std@0.135.0+) library implementation, no work-around is possible due to: 1. being impossible to actually revoke a permission (`revoke` just "downgrade[s] a permission from 'granted' to 'prompt'); 2. the permission API has no synchronous functions making revocation impossible prior to a synchronous access (even if a polyfill patch via module was attempted). Reversion and pinning of deno::std to v0.134.0 avoids the patch changing from async gating to sync `try...catch` while still maintaining most of the functionality of later deno::std library versions. # refs [1] [fix(node): Make global.ts evaluate synchronously](denoland/std#2098) [2] [Execution order of imports in deno is unclear/unexpected](denoland/deno#14243) [3] [std/node should avoid TLA](denoland/std#2097) ## related discussion/issues [Discussion ~ Bring back permission prompt behind a flag](denoland/deno#3811) [Security prompt by default (instead of throw)](denoland/deno#10183) [Seeking a better UX for permissions](denoland/deno#11061) [Design Meeting 2021-07-29 ~ `Prompt by default`](denoland/deno#11767) [permission prompt problems](denoland/deno#11936) [`deno repl` has permissions by default?](denoland/deno#12665) [Bad UX with prompt by default](denoland/deno#13730) [DENO_NO_PROMPT env var support](denoland/deno#14208) [feat: Add DENO_NO_PROMPT variable](denoland/deno#14209) [HowTO test that a module is *no-panic* and *no-prompt* when statically imported?](denoland/deno#15356)
rivy
added a commit
to rivy/js.xdg-portable
that referenced
this pull request
Aug 9, 2022
…mpt(s) - revert to earlier deno::std library version without NODE_DEBUG permission prompts # [why] In the change from deno::std@0.134.0 to deno::std@0.135.0, a change was made to remove a top-level async permission query gate from the library, replacing it with a synchronous `try...catch`.[1] This was done to avoid unexpected module load order since the module with `await` will run at a time later than synchronous peers (although still prior to user code).[2,3] Unfortunately, the, now default, behavior of Deno is to prompt on use for permissions which aren't 'granted' or 'denied'. This leads to ugly UI/UX for scripts which try to control their logging and UI (as well as being confusing and unexpected for non-developer users). This UI/UX behavior is only avoidable if the script is run with the `--no-prompt` option flag or all needed permissions enabled, encouraging use of `--no-prompt` and, more problematically, `--allow-all`. As of 2022-08, with the current Deno (Deno v1.8.0+) and deno::std (deno::std@0.135.0+) library implementation, no work-around is possible due to: 1. being impossible to actually revoke a permission (`revoke` just "downgrade[s] a permission from 'granted' to 'prompt'); 2. the permission API has no synchronous functions making revocation impossible prior to a synchronous access (even if a polyfill patch via module was attempted). Reversion and pinning of deno::std to v0.134.0 avoids the patch changing from async gating to sync `try...catch` while still maintaining most of the functionality of later deno::std library versions. # refs [1] [fix(node): Make global.ts evaluate synchronously](denoland/std#2098) [2] [Execution order of imports in deno is unclear/unexpected](denoland/deno#14243) [3] [std/node should avoid TLA](denoland/std#2097) ## related discussion/issues [Discussion ~ Bring back permission prompt behind a flag](denoland/deno#3811) [Security prompt by default (instead of throw)](denoland/deno#10183) [Seeking a better UX for permissions](denoland/deno#11061) [Design Meeting 2021-07-29 ~ `Prompt by default`](denoland/deno#11767) [permission prompt problems](denoland/deno#11936) [`deno repl` has permissions by default?](denoland/deno#12665) [Bad UX with prompt by default](denoland/deno#13730) [DENO_NO_PROMPT env var support](denoland/deno#14208) [feat: Add DENO_NO_PROMPT variable](denoland/deno#14209) [HowTO test that a module is *no-panic* and *no-prompt* when statically imported?](denoland/deno#15356)
rivy
added a commit
to rivy/js.xdg-app-paths
that referenced
this pull request
Aug 10, 2022
…mpt(s) - revert to earlier deno::std library version without NODE_DEBUG permission prompts # [why] In the change from deno::std@0.134.0 to deno::std@0.135.0, a change was made to remove a top-level async permission query gate from the library, replacing it with a synchronous `try...catch`.[1] This was done to avoid unexpected module load order since the module with `await` will run at a time later than synchronous peers (although still prior to user code).[2,3] Unfortunately, the, now default, behavior of Deno is to prompt on use for permissions which aren't 'granted' or 'denied'. This leads to ugly UI/UX for scripts which try to control their logging and UI (as well as being confusing and unexpected for non-developer users). This UI/UX behavior is only avoidable if the script is run with the `--no-prompt` option flag or all needed permissions enabled, encouraging use of `--no-prompt` and, more problematically, `--allow-all`. As of 2022-08, with the current Deno (Deno v1.8.0+) and deno::std (deno::std@0.135.0+) library implementation, no work-around is possible due to: 1. being impossible to actually revoke a permission (`revoke` just "downgrade[s] a permission from 'granted' to 'prompt'); 2. the permission API has no synchronous functions making revocation impossible prior to a synchronous access (even if a polyfill patch via module was attempted). Reversion and pinning of deno::std to v0.134.0 avoids the patch changing from async gating to sync `try...catch` while still maintaining most of the functionality of later deno::std library versions. # refs [1] [fix(node): Make global.ts evaluate synchronously](denoland/std#2098) [2] [Execution order of imports in deno is unclear/unexpected](denoland/deno#14243) [3] [std/node should avoid TLA](denoland/std#2097) ## related discussion/issues [Discussion ~ Bring back permission prompt behind a flag](denoland/deno#3811) [Security prompt by default (instead of throw)](denoland/deno#10183) [Seeking a better UX for permissions](denoland/deno#11061) [Design Meeting 2021-07-29 ~ `Prompt by default`](denoland/deno#11767) [permission prompt problems](denoland/deno#11936) [`deno repl` has permissions by default?](denoland/deno#12665) [Bad UX with prompt by default](denoland/deno#13730) [DENO_NO_PROMPT env var support](denoland/deno#14208) [feat: Add DENO_NO_PROMPT variable](denoland/deno#14209) [HowTO test that a module is *no-panic* and *no-prompt* when statically imported?](denoland/deno#15356)
rivy
added a commit
to rivy/js.xdg-app-paths
that referenced
this pull request
Aug 10, 2022
…mpt(s) - revert to earlier deno::std library version without NODE_DEBUG permission prompts # [why] In the change from deno::std@0.134.0 to deno::std@0.135.0, a change was made to remove a top-level async permission query gate from the library, replacing it with a synchronous `try...catch`.[1] This was done to avoid unexpected module load order since the module with `await` will run at a time later than synchronous peers (although still prior to user code).[2,3] Unfortunately, the, now default, behavior of Deno is to prompt on use for permissions which aren't 'granted' or 'denied'. This leads to ugly UI/UX for scripts which try to control their logging and UI (as well as being confusing and unexpected for non-developer users). This UI/UX behavior is only avoidable if the script is run with the `--no-prompt` option flag or all needed permissions enabled, encouraging use of `--no-prompt` and, more problematically, `--allow-all`. As of 2022-08, with the current Deno (Deno v1.8.0+) and deno::std (deno::std@0.135.0+) library implementation, no work-around is possible due to: 1. being impossible to actually revoke a permission (`revoke` just "downgrade[s] a permission from 'granted' to 'prompt'); 2. the permission API has no synchronous functions making revocation impossible prior to a synchronous access (even if a polyfill patch via module was attempted). Reversion and pinning of deno::std to v0.134.0 avoids the patch changing from async gating to sync `try...catch` while still maintaining most of the functionality of later deno::std library versions. # refs [1] [fix(node): Make global.ts evaluate synchronously](denoland/std#2098) [2] [Execution order of imports in deno is unclear/unexpected](denoland/deno#14243) [3] [std/node should avoid TLA](denoland/std#2097) ## related discussion/issues [Discussion ~ Bring back permission prompt behind a flag](denoland/deno#3811) [Security prompt by default (instead of throw)](denoland/deno#10183) [Seeking a better UX for permissions](denoland/deno#11061) [Design Meeting 2021-07-29 ~ `Prompt by default`](denoland/deno#11767) [permission prompt problems](denoland/deno#11936) [`deno repl` has permissions by default?](denoland/deno#12665) [Bad UX with prompt by default](denoland/deno#13730) [DENO_NO_PROMPT env var support](denoland/deno#14208) [feat: Add DENO_NO_PROMPT variable](denoland/deno#14209) [HowTO test that a module is *no-panic* and *no-prompt* when statically imported?](denoland/deno#15356)
rivy
added a commit
to rivy/js.xdg-app-paths
that referenced
this pull request
Aug 10, 2022
…mpt(s) - revert to earlier deno::std library version without NODE_DEBUG permission prompts # [why] In the change from deno::std@0.134.0 to deno::std@0.135.0, a change was made to remove a top-level async permission query gate from the library, replacing it with a synchronous `try...catch`.[1] This was done to avoid unexpected module load order since the module with `await` will run at a time later than synchronous peers (although still prior to user code).[2,3] Unfortunately, the, now default, behavior of Deno is to prompt on use for permissions which aren't 'granted' or 'denied'. This leads to ugly UI/UX for scripts which try to control their logging and UI (as well as being confusing and unexpected for non-developer users). This UI/UX behavior is only avoidable if the script is run with the `--no-prompt` option flag or all needed permissions enabled, encouraging use of `--no-prompt` and, more problematically, `--allow-all`. As of 2022-08, with the current Deno (Deno v1.8.0+) and deno::std (deno::std@0.135.0+) library implementation, no work-around is possible due to: 1. being impossible to actually revoke a permission (`revoke` just "downgrade[s] a permission from 'granted' to 'prompt'); 2. the permission API has no synchronous functions making revocation impossible prior to a synchronous access (even if a polyfill patch via module was attempted). Reversion and pinning of deno::std to v0.134.0 avoids the patch changing from async gating to sync `try...catch` while still maintaining most of the functionality of later deno::std library versions. # refs [1] [fix(node): Make global.ts evaluate synchronously](denoland/std#2098) [2] [Execution order of imports in deno is unclear/unexpected](denoland/deno#14243) [3] [std/node should avoid TLA](denoland/std#2097) ## related discussion/issues [Discussion ~ Bring back permission prompt behind a flag](denoland/deno#3811) [Security prompt by default (instead of throw)](denoland/deno#10183) [Seeking a better UX for permissions](denoland/deno#11061) [Design Meeting 2021-07-29 ~ `Prompt by default`](denoland/deno#11767) [permission prompt problems](denoland/deno#11936) [`deno repl` has permissions by default?](denoland/deno#12665) [Bad UX with prompt by default](denoland/deno#13730) [DENO_NO_PROMPT env var support](denoland/deno#14208) [feat: Add DENO_NO_PROMPT variable](denoland/deno#14209) [HowTO test that a module is *no-panic* and *no-prompt* when statically imported?](denoland/deno#15356)
rivy
added a commit
to rivy/js.xdg-app-paths
that referenced
this pull request
Aug 13, 2022
…mpt(s) - revert to earlier deno::std library version without NODE_DEBUG permission prompts # [why] In the change from deno::std@0.134.0 to deno::std@0.135.0, a change was made to remove a top-level async permission query gate from the library, replacing it with a synchronous `try...catch`.[1] This was done to avoid unexpected module load order since the module with `await` will run at a time later than synchronous peers (although still prior to user code).[2,3] Unfortunately, the, now default, behavior of Deno is to prompt on use for permissions which aren't 'granted' or 'denied'. This leads to ugly UI/UX for scripts which try to control their logging and UI (as well as being confusing and unexpected for non-developer users). This UI/UX behavior is only avoidable if the script is run with the `--no-prompt` option flag or all needed permissions enabled, encouraging use of `--no-prompt` and, more problematically, `--allow-all`. As of 2022-08, with the current Deno (Deno v1.8.0+) and deno::std (deno::std@0.135.0+) library implementation, no work-around is possible due to: 1. being impossible to actually revoke a permission (`revoke` just "downgrade[s] a permission from 'granted' to 'prompt'); 2. the permission API has no synchronous functions making revocation impossible prior to a synchronous access (even if a polyfill patch via module was attempted). Reversion and pinning of deno::std to v0.134.0 avoids the patch changing from async gating to sync `try...catch` while still maintaining most of the functionality of later deno::std library versions. # refs [1] [fix(node): Make global.ts evaluate synchronously](denoland/std#2098) [2] [Execution order of imports in deno is unclear/unexpected](denoland/deno#14243) [3] [std/node should avoid TLA](denoland/std#2097) ## related discussion/issues [Discussion ~ Bring back permission prompt behind a flag](denoland/deno#3811) [Security prompt by default (instead of throw)](denoland/deno#10183) [Seeking a better UX for permissions](denoland/deno#11061) [Design Meeting 2021-07-29 ~ `Prompt by default`](denoland/deno#11767) [permission prompt problems](denoland/deno#11936) [`deno repl` has permissions by default?](denoland/deno#12665) [Bad UX with prompt by default](denoland/deno#13730) [DENO_NO_PROMPT env var support](denoland/deno#14208) [feat: Add DENO_NO_PROMPT variable](denoland/deno#14209) [HowTO test that a module is *no-panic* and *no-prompt* when statically imported?](denoland/deno#15356)
rivy
added a commit
to rivy/js.xdg-app-paths
that referenced
this pull request
Aug 14, 2022
…mpt(s) - revert to earlier deno::std library version without NODE_DEBUG permission prompts # [why] In the change from deno::std@0.134.0 to deno::std@0.135.0, a change was made to remove a top-level async permission query gate from the library, replacing it with a synchronous `try...catch`.[1] This was done to avoid unexpected module load order since the module with `await` will run at a time later than synchronous peers (although still prior to user code).[2,3] Unfortunately, the, now default, behavior of Deno is to prompt on use for permissions which aren't 'granted' or 'denied'. This leads to ugly UI/UX for scripts which try to control their logging and UI (as well as being confusing and unexpected for non-developer users). This UI/UX behavior is only avoidable if the script is run with the `--no-prompt` option flag or all needed permissions enabled, encouraging use of `--no-prompt` and, more problematically, `--allow-all`. As of 2022-08, with the current Deno (Deno v1.8.0+) and deno::std (deno::std@0.135.0+) library implementation, no work-around is possible due to: 1. being impossible to actually revoke a permission (`revoke` just "downgrade[s] a permission from 'granted' to 'prompt'); 2. the permission API has no synchronous functions making revocation impossible prior to a synchronous access (even if a polyfill patch via module was attempted). Reversion and pinning of deno::std to v0.134.0 avoids the patch changing from async gating to sync `try...catch` while still maintaining most of the functionality of later deno::std library versions. # refs [1] [fix(node): Make global.ts evaluate synchronously](denoland/std#2098) [2] [Execution order of imports in deno is unclear/unexpected](denoland/deno#14243) [3] [std/node should avoid TLA](denoland/std#2097) ## related discussion/issues [Discussion ~ Bring back permission prompt behind a flag](denoland/deno#3811) [Security prompt by default (instead of throw)](denoland/deno#10183) [Seeking a better UX for permissions](denoland/deno#11061) [Design Meeting 2021-07-29 ~ `Prompt by default`](denoland/deno#11767) [permission prompt problems](denoland/deno#11936) [`deno repl` has permissions by default?](denoland/deno#12665) [Bad UX with prompt by default](denoland/deno#13730) [DENO_NO_PROMPT env var support](denoland/deno#14208) [feat: Add DENO_NO_PROMPT variable](denoland/deno#14209) [HowTO test that a module is *no-panic* and *no-prompt* when statically imported?](denoland/deno#15356)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #14208.