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

Fix CLI formatting on windows #119435

Merged
merged 7 commits into from
Nov 23, 2021

Conversation

thomheymann
Copy link
Contributor

This PR fixes CLI command formatting on windows

Screenshot 2021-11-23 at 10 22 03

.

@thomheymann thomheymann added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes auto-backport Deprecated - use backport:version if exact versions are needed v8.1.0 Feature:Security/Interactive Setup Platform Security - Interactive setup mode labels Nov 23, 2021
@thomheymann thomheymann requested a review from a team November 23, 2021 10:22
@azasypkin azasypkin self-requested a review November 23, 2021 10:27
@azasypkin
Copy link
Member

ACK: reviewing...

Copy link
Member

@azasypkin azasypkin left a comment

Choose a reason for hiding this comment

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

Looks and works as expected, thanks!

* Side Public License, v 1.
*/

export function formatBashCommand(command: string, args?: string) {
Copy link
Member

Choose a reason for hiding this comment

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

optional nit: It's not a big deal at all, just wanted to note: it sounds like too much credit to Bash shell here ;) Maybe something like formatCommadLineSnippet\formatCli... would be more OS/shell agnostic.


export function formatBashCommand(command: string, args?: string) {
const isWindows = window.navigator.userAgent.includes('Win');
return `${isWindows ? `bin\\${command}.bat` : `bin/${command}`}${args ? ` ${args}` : ''}`;
Copy link
Member

Choose a reason for hiding this comment

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

note: it's great that we have this code in one place now 🎉 If users ever complain about false positives here we'll need to update just one place (e.g. fetch actual OS from the server).

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Test Failures

  • [job] [logs] Default CI Group #26 / machine learning anomaly detection custom urls tests Discover type custom URL opens Discover page from test link in the edit job flyout

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
interactiveSetup 44 45 +1

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
interactiveSetup 57.6KB 57.6KB +44.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@thomheymann thomheymann merged commit f58cd5c into elastic:main Nov 23, 2021
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Nov 23, 2021
* Provide more guidance on enrollment token

* Added suggestions from copy review

* Simplify copy

* format bash commands correctly

* Added suggestions from code review
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
8.0

This backport PR will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Nov 23, 2021
* Provide more guidance on enrollment token

* Added suggestions from copy review

* Simplify copy

* format bash commands correctly

* Added suggestions from code review

Co-authored-by: Thom Heymann <[email protected]>
dmlemeshko pushed a commit that referenced this pull request Nov 29, 2021
* Provide more guidance on enrollment token

* Added suggestions from copy review

* Simplify copy

* format bash commands correctly

* Added suggestions from code review
TinLe pushed a commit to TinLe/kibana that referenced this pull request Dec 22, 2021
* Provide more guidance on enrollment token

* Added suggestions from copy review

* Simplify copy

* format bash commands correctly

* Added suggestions from code review
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed Feature:Security/Interactive Setup Platform Security - Interactive setup mode release_note:skip Skip the PR/issue when compiling release notes v8.0.0 v8.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants