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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ 馃Ч ] Editorconfig , Querystring Replacement , Shorter Test Video #9

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

PhoneDroid
Copy link

@PhoneDroid PhoneDroid commented Jun 30, 2023

( Check the inidivual commits for changes instead of the File Changes tab -> editorconfig was applied to all files )

  • Added editorconfig for basic consistency
  • Applied editorconfig to all files
  • Replaced deprecated querystring library with standard URLSearchParams API
  • Replaced test video for shorter + much smaller filesize one

closes #10

@@ -416,7 +416,7 @@ const getVideoInfoPage = async (id: string, options: GetInfoOptions) => {
url.searchParams.set("cver", `7${cver.substr(1)}`);
url.searchParams.set("html5", "1");
let body = await request(url.toString(), options).then((e) => e.text());
let info = querystring.decode(body);
let info = Object.fromEntries([ ... new URLSearchParams(body).entries() ]);
Copy link
Owner

Choose a reason for hiding this comment

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

I think you can simply do Object.fromEntries(new URLSearchParams(body))

.map((e: any) => qs.parse(e));
.map((e: any) => new URLSearchParams(e))
.map(( params ) => [ ... params.entries() ] )
.map(( params ) => Object.fromEntries(params) );
Copy link
Owner

Choose a reason for hiding this comment

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

FYI this is an array, not a lazy iterator where chained maps can combine for less time complexity. This is going to iterate over the params 3 * n times. Just do

.map((e: any) => Object.fromEntries(new URLSearchParams(e)))


[*.md]

trim_trailing_whitespace = false
Copy link
Owner

Choose a reason for hiding this comment

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

What about this instead? I think the comment at top is unnecessary and the ident size should be 2.

root = true

[*]
end_of_line = lf
insert_final_newline = true
indent_style = space
indent_size = 2
charset = utf-8
trim_trailing_whitespace = true

@DjDeveloperr
Copy link
Owner

Thanks for the PR! After it is landed, I will setup formatting & linting via deno fmt and deno lint in CI.

Unrelated but I recommend making PRs from non-main branch, so it allows maintainers to make changes to the PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants