10000 scripts: fix generate-config-doc, handle usage errors by flotwig · Pull Request #4807 · thelounge/thelounge · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

scripts: fix generate-config-doc, handle usage errors #4807

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

Merged

Conversation

flotwig
Copy link
Contributor
@flotwig flotwig commented Dec 12, 2023

When trying to update the docs for #1344 I noticed that the generate-config-doc script was broken because it was no longer correctly importing server/log, my guess is it just got missed during TS conversion. I fixed the default import in this PR.

This PR also fixes the types in generate-config-doc.js, which are checked because checkJs is enabled in tsconfig.json. I did not convert the file to TypeScript because it is not part of the eslint TS config and I was unsure about making changes to the lint setup. The only type change required was marking acc as string[] instead of never[].

Since generate-config-doc requires TS imports, the comments suggesting to use node ./scripts/... to execute it are wrong. I switched the comments to use a npm script which runs ts-node for the user.

Additionally, I added a check that the DOC_ROOT_PATH is supplied, so the script exits with a nice error instead of a less useful uncaught exception from writeFileSync.

Running this revealed that some of the existing config docs are out-of-date, so I opened a PR to sync up: thelounge/thelounge.github.io#275

Comment on lines +32 to +33
/** @type {string[]} */
const acc = [];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't use JSDoc type annotations for TypeScript often. If there is a way to do this more concisely without separately defining and annotating acc like this, it would be better imo. I didn't/couldn't just convert this file to .ts due to reasons explained in the PR comment.

@flotwig flotwig force-pushed the fixup-generate-config-docs-script branch from 3a59475 to 6603c1a Compare December 12, 2023 16:52
@brunnre8 brunnre8 added the Status: needs-review PR not yet reviewed by enough maintainers label Dec 13, 2023
Copy link
Member
@MaxLeiter MaxLeiter left a comment

Choose a reason for hiding this comment

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

Thanks!

@MaxLeiter MaxLeiter removed the Status: needs-review PR not yet reviewed by enough maintainers label Dec 27, 2023
@MaxLeiter MaxLeiter merged commit 01cfe3d into thelounge:master Dec 27, 2023
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.

3 participants
0