Skip to content

feat(core): Add 'Get-Encoding()' function to fix missing webClient encoding#4956

Merged
niheaven merged 5 commits intoScoopInstaller:developfrom
yi-Xu-0100:fix-encoding
Jun 13, 2022
Merged

feat(core): Add 'Get-Encoding()' function to fix missing webClient encoding#4956
niheaven merged 5 commits intoScoopInstaller:developfrom
yi-Xu-0100:fix-encoding

Conversation

@yi-Xu-0100
Copy link
Copy Markdown
Contributor

@yi-Xu-0100 yi-Xu-0100 commented May 28, 2022

Add Get-Encoding function in core.ps1

Description

Motivation and Context

Closes #4911 #4324

Relates to #4911 #4324

How Has This Been Tested?

Checklist:

  • I have read the Contributing Guide.
  • I have ensured that I am targeting the develop branch.
  • I have updated the documentation accordingly.
  • I have updated the tests accordingly.
  • I have added an entry in the CHANGELOG.

yi-Xu-0100 added a commit to yi-Xu-0100/Scoop that referenced this pull request May 28, 2022
@yi-Xu-0100 yi-Xu-0100 changed the title fix(core): use Get-Encoding to set webclient encoding fix(core): add Get-Encoding function to fix missing webClient encoding May 28, 2022
@yi-Xu-0100
Copy link
Copy Markdown
Contributor Author

@rashil2000 Any review here will be helpful~ 😁

Comment thread lib/autoupdate.ps1 Outdated
Comment thread lib/core.ps1 Outdated
Comment thread lib/manifest.ps1 Outdated
Comment thread libexec/scoop-virustotal.ps1 Outdated
@yi-Xu-0100 yi-Xu-0100 requested a review from niheaven June 3, 2022 09:44
yi-Xu-0100 added a commit to yi-Xu-0100/Scoop that referenced this pull request Jun 3, 2022
@niheaven
Copy link
Copy Markdown
Member

niheaven commented Jun 4, 2022

Do you have some testing manifests?

@yi-Xu-0100
Copy link
Copy Markdown
Contributor Author

@niheaven ,Thanks for your reply. 😁

In #4911 (comment) , I use

scoop install "https://raw.githubusercontent.com/ygguorun/scoop-bucket/master/bucket/PDFPatcher.json"

, and this manifest - DoveBoy/Apps/pdfpatcher has the checkver, and they all work well.

@niheaven
Copy link
Copy Markdown
Member

niheaven commented Jun 6, 2022

checkver has some error, could you please check it again?

@yi-Xu-0100
Copy link
Copy Markdown
Contributor Author

@niheaven I used it for scoop update and scoop install, they all work well.

Could you please provider the way to test the checkver, I have no idea for it.🤔

@niheaven
Copy link
Copy Markdown
Member

niheaven commented Jun 9, 2022

Checkout this branch and cd in any bucket, run .\bin\checkver.ps1 xxx, there are some bugs :)

yi-Xu-0100 added a commit to yi-Xu-0100/Scoop that referenced this pull request Jun 9, 2022
…ing (ScoopInstaller#4956)

Add `Get-Encoding` function in core.ps1

closes: ScoopInstaller#4911 ScoopInstaller#4324

Co-authored-by: Hsiao-nan Cheung <niheaven@gmail.com>
…ing (ScoopInstaller#4956)

Add `Get-Encoding` function in core.ps1

closes: ScoopInstaller#4911 ScoopInstaller#4324

Co-authored-by: Hsiao-nan Cheung <niheaven@gmail.com>
@yi-Xu-0100
Copy link
Copy Markdown
Contributor Author

@niheaven Thanks for pointing out the problem, I think it's fixed now. 😁

@niheaven
Copy link
Copy Markdown
Member

Some tweaks, and please test it @yi-Xu-0100

niheaven
niheaven previously approved these changes Jun 10, 2022
@niheaven niheaven changed the title fix(core): add Get-Encoding function to fix missing webClient encoding feat(core): Add 'Get-Encoding()' function to fix missing webClient encoding Jun 10, 2022
@yi-Xu-0100
Copy link
Copy Markdown
Contributor Author

@niheaven Thanks for adding the judgment for null values, but there is something we need to revert.

If we use (Get-Encoding($wc)).GetString($wc.DownloadData($url)), then (Get-Encoding($wc)) can only get the return result in UTF-8, because The ResponseHeaders of $wc will be $null before the data is downloaded.

So I re-split the expression to handle the case where charset is something else.

@yi-Xu-0100 yi-Xu-0100 requested a review from niheaven June 11, 2022 02:35
@niheaven
Copy link
Copy Markdown
Member

Aha, right?

Okay, I'll test it tomorrow and it seems stable enough now.

@niheaven niheaven merged commit 6e25e44 into ScoopInstaller:develop Jun 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants