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

invoke-webrequest: add page #8177

Merged
merged 14 commits into from
Jul 12, 2022
Merged

invoke-webrequest: add page #8177

merged 14 commits into from
Jul 12, 2022

Conversation

reinhart1010
Copy link
Collaborator

  • The page (if new), does not already exist in the repository.
  • The page is in the correct platform directory (common/, linux/, etc.)
  • The page has 8 or fewer examples.
  • The PR title conforms to the recommended templates.
  • The page follows the content guidelines.
  • The page description includes a link to documentation or a homepage (if applicable).

Version of the command being documented (if known): 7.2.1

After quite a long hiatus I decided to contribute some more on PowerShell (and soon for Indonesian translations for macOS commands).

We have talked a lot about Invoke-WebRequest and "incompatible aliases" when curl and wget are not installed in Windows. So I decided to create a special alias page for them, with a command to check which program (curl/wget or Invoke-WebRequest) was used to evaluate the curl or wget command, despite I'm afraid they won't pass the linter:

Check whether wget is properly installed by printing its version number. If this command evaluates into an error, PowerShell may have substituted this command with Invoke-WebRequest

@github-actions github-actions bot added the new command Issues requesting creation of a new page or PRs adding a new page for a command. label Jul 2, 2022
@tldr-bot
Copy link

tldr-bot commented Jul 2, 2022

Hello! I've noticed something unusual when checking this PR:

  • The page windows/curl.md already exists under the common platform.
  • The page windows/wget.md already exists under the common platform.

Is this intended? If so, just ignore this comment. Otherwise, please double-check the commits.

@tldr-bot

This comment was marked as outdated.

@navarroaxel navarroaxel changed the title invoke-webrequest: Add page invoke-webrequest: add page Jul 3, 2022
Copy link
Member

@sbrl sbrl left a comment

Choose a reason for hiding this comment

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

Thanks for the new page! I've left some comments below for you to review.

@@ -0,0 +1,15 @@
# curl
Copy link
Member

Choose a reason for hiding this comment

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

This will overwrite the existing curl page in the common platform.

@@ -0,0 +1,15 @@
# wget
Copy link
Member

Choose a reason for hiding this comment

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

Same here. Let's remove these wget and curl pages from this pull request.

Copy link
Collaborator Author

@reinhart1010 reinhart1010 Jul 3, 2022

Choose a reason for hiding this comment

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

I wrote these two pages for two reasons. First is the fact that in PowerShell for Windows, both commands will be substituted with Invoke-WebRequest with incompatible set of accepted parameters when wget or curl is not visible on %PATH%:

image

and similarly, executing curl and wget without additional subcommand or parameters will still return the same prompt with Invoke-WebRequest if both aren't installed on %PATH%:

image

As far as I know there are some edge cases where a command documentation is split between common and OS-specific versions, e.g. common/cd and windows/cd. As long as tldr clients still can show the common or platform-specific versions of the same command I still prefer to keep windows/curl and windows/wget this way to avoid confusion among those PowerShell users.

Ah I forgot that the original documentation should be accessed with tldr {{curl|wget}} --platform common instead of tldr common/{{curl|wget}}.

Copy link
Member

@sbrl sbrl Jul 12, 2022

Choose a reason for hiding this comment

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

Oh I see, thanks for clarifying this. This is a rather confusing situation, isn't it?

  • cmd.exe is the main shell for Windows, which if you have curl or wget installed works as on Linux
  • PowerShell is also an official shell distributed and enabled by default on Windows, where curl and wget resolve → Invoke-WebRequest
  • Someone should complain to Microsoft about this awfully confusing behaviour, since curl, wget, and Invoke-WebRequest all have incompatible arguments

With this in mind, your solution here seems to be the best option. Thanks for explaining - not being a Windows user I didn't understand at first.

Copy link
Collaborator

@marchersimon marchersimon left a comment

Choose a reason for hiding this comment

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

Not entirely sure about the curl/wgetpage, but the content looks fine.
It's not the most elegant solution, but I think the only option we have.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new command Issues requesting creation of a new page or PRs adding a new page for a command.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants