The first MVP build #2
Conversation
…th daily-stars-explorer.
WalkthroughThis pull request introduces multiple configuration files (Clasp, environment, CI/CD, gitignore, Prettier, TypeScript, and Vite), along with updated documentation and package metadata for the awesome-things project. New modules cover GitHub API interactions, URL utilities, markdown parsing, and Google Apps Script handlers for custom menu and spreadsheet operations. Additional TypeScript declaration files, ESLint configuration, and a comprehensive test suite are added, enhancing development quality and integration with Google Sheets and GitHub data. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant GS as Google Sheets UI
participant AS as Apps Script (src/index.ts)
participant H as Handlers Module
U->>GS: Open Spreadsheet
GS->>AS: Trigger onOpen()
AS->>GS: Add "Awesome Things" menu
U->>GS: Select "Load Top Content"
GS->>AS: Invoke loadTopOfContent()
AS->>H: Call handlers.loadTopOfContent() with repo URL
H->>GS: Update sheet with Table of Contents and content
Poem
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (30)
eslint.config.js (1)
13-15: Consider enabling unused variable checks in the future.The
@typescript-eslint/no-unused-varsrule has been disabled. While this might be helpful during early development to reduce noise, consider enabling it later to prevent code bloat and potential bugs.rules: { - '@typescript-eslint/no-unused-vars': 'off', + '@typescript-eslint/no-unused-vars': ['warn', { + argsIgnorePattern: '^_', + varsIgnorePattern: '^_' + }], },This alternative configuration would still allow for intentionally unused variables when prefixed with underscore.
.clasp.json.example (1)
1-6: Consider adding comments to explain configuration fields.Since this is an example file meant to guide users, adding comments to explain each field's purpose would improve clarity for developers who might be unfamiliar with CLASP.
{ + // Your Google Apps Script project ID (required) "scriptId": "", + // Directory containing files that should be pushed to Google Apps Script "rootDir": "./dist", + // File extension of the files to push "fileExtension": "js", + // Optional array to specify the order in which files should be pushed "filePushOrder": [] }src/extra.ts (2)
1-6: Add documentation and improve function parameter clarity.The
run()function correctly accesses the active spreadsheet, but lacks documentation explaining its purpose and usage. The empty string parameter passed toloadSection()is not self-explanatory.Consider adding JSDocs and using a more descriptive parameter:
+/** + * Entry point function for running the section loader from the spreadsheet. + * This function is exposed to Google Apps Script and can be called directly. + */ function run() { const spreadsheet = SpreadsheetApp.getActiveSpreadsheet() // @ts-expect-error no import, because of apps script building - loadSection(spreadsheet, '') + // Load the root section (empty string represents the root) + loadSection(spreadsheet, '') }
4-5: Consider alternative approach to handle type checking.The
@ts-expect-errorcomment is used to suppress TypeScript errors due to the absence of imports, which is necessary for Google Apps Script's build process.For better type safety, consider using an approach with namespace declarations or module patterns that would allow TypeScript to recognize the function without requiring imports:
// In a handlers.d.ts file: declare function loadSection(spreadsheet: GoogleAppsScript.Spreadsheet.Spreadsheet, section: string): void; // Then in this file: function run() { const spreadsheet = SpreadsheetApp.getActiveSpreadsheet(); loadSection(spreadsheet, ''); // No type error }This maintains type safety while respecting Google Apps Script's limitations on imports.
vite.config.ts (1)
6-30: Reconsider disabled optimizations for production buildsThe configuration disables several important optimizations:
- Tree-shaking (line 26)
- Minification (line 28)
- Strict mode (line 22)
While these choices might be appropriate for development or debugging, they will result in larger bundle sizes and potentially introduce subtle bugs in production.
Consider adding environment-specific configuration:
export default defineConfig({ build: { lib: { entry: { index: path.resolve(__dirname, 'src/index.ts'), extra: path.resolve(__dirname, 'src/extra.ts'), }, formats: ['cjs'], }, rollupOptions: { output: { dir: 'dist', entryFileNames: ({ name }: PreRenderedChunk) => { return `${name}.js` }, format: 'cjs', - strict: false, + strict: process.env.NODE_ENV !== 'development', exports: 'none', minifyInternalExports: false, }, - treeshake: false, + treeshake: process.env.NODE_ENV !== 'development', }, - minify: false, + minify: process.env.NODE_ENV !== 'development', emptyOutDir: true, }, // ... })README.md (6)
5-5: Consider revising for clarity and grammatical correctness.The sentence structure needs improvement to clearly convey the project's purpose.
-**awesome-things** helps you organize all the options in one table, compare the numbers of stars, open/closed issues/PRs, track the project's dynamic and ultimately make an informed which solution to pick. +**awesome-things** helps you organize all the options in one table, compare the numbers of stars, open/closed issues/PRs, track the project's dynamics and ultimately make an informed decision about which solution to pick.
22-22: Minor typo in setup instructions.There's a typographical error in the clasp installation instruction.
-2. Install [clasp](https://github.com/google/clasp) and grand access +2. Install [clasp](https://github.com/google/clasp) and grant access
53-53: Typo in environment file creation step.There's a minor typo in the instruction for creating the environment file.
- 2. Crate a `.env.local` + 2. Create a `.env.local`
59-59: Missing article in instruction.According to the static analysis hint, there's a missing article in this instruction.
- 3. Add the GitHub token to `.env.local` file + 3. Add the GitHub token to the `.env.local` file🧰 Tools
🪛 LanguageTool
[uncategorized] ~59-~59: You might be missing the article “the” here.
Context: ... ``` 3. Add the GitHub token to.env.localfile 5. (Optional) Integr...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
65-67: Missing language specifier in code block.The markdown code block should specify a language for proper syntax highlighting.
- ``` + ```env VITE_STARS_EXPLORER_URL=127.0.0.1:8080 ```🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
65-65: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
78-78: Missing article in deployment instruction.According to the static analysis hint, there's a missing article in this instruction.
-6. Deploy the project with following command: +6. Deploy the project with the following command:🧰 Tools
🪛 LanguageTool
[uncategorized] ~78-~78: You might be missing the article “the” here.
Context: ...t6. Deploy the project with following command:shell npm run deploy...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
tests/parser/__mocks__/dummy.md (1)
107-107: Invalid link fragment in reference.The markdown linter detected an invalid link fragment in this line. The fragment
#necmight not correctly resolve in your parser tests.-> 💡 EOS opprobrium eos floret si [NAM - Recusandae](#nec). +> 💡 EOS opprobrium eos floret si [NAM - Recusandae](#header-1-1).🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
107-107: Link fragments should be valid
null(MD051, link-fragments)
.github/workflows/ci.yml (2)
18-21: Consider using a consistent Node.js version across jobs.You're using Node.js 18 for linting but Node.js 22 for testing and building. Using different versions could potentially lead to inconsistencies.
- node-version: 18 + node-version: 22
52-70: Consider adding an artifact upload step for the build job.After building the project, it would be useful to upload the built artifacts for deployment or further testing.
- name: Build the project run: npm run build + + - name: Upload build artifacts + uses: actions/upload-artifact@v4 + with: + name: build-artifacts + path: dist/tsconfig.json (1)
21-22: Consider enablingnoUnusedLocalsfor cleaner code.You've disabled
noUnusedLocalsbut enablednoUnusedParameters. For consistency and to avoid dead code, consider enabling both.- "noUnusedLocals": false, + "noUnusedLocals": true,src/handlers/toc.ts (4)
1-2: Add JSDoc comments to describe the class purpose.The
ToCBuilderclass could benefit from documentation that explains its purpose and how it relates to the table of contents extraction.+/** + * Builds a structured table of contents from parsed markdown headers. + * Creates a 2D array suitable for populating a spreadsheet. + */ export class ToCBuilder { private things: { name: string; offset: number; total: number }[] = []
4-8: Add JSDoc comments for the method and include type annotations for return value.The
addThingmethod should be documented and its return type explicitly annotated.+ /** + * Adds an item to the table of contents. + * @param name The header text + * @param offset The header level (1 = H1, 2 = H2, etc.) + * @param total The count or metric associated with this header + * @returns The current ToCBuilder instance for method chaining + */ - addThing(name: string, offset: number, total: number) { + addThing(name: string, offset: number, total: number): ToCBuilder { this.things.push({ name, offset, total }) return this }
14-16: Use a more descriptive variable name instead of generic "header".The variable
headercould be named more specifically asheaderRowto better describe its purpose.- const header = new Array(columns - 1).fill('').map((_, i) => `Header ${i + 1}`) - header.push('Total') - result.push(header) + const headerRow = new Array(columns - 1).fill('').map((_, i) => `Header ${i + 1}`) + headerRow.push('Total') + result.push(headerRow)
18-23: Consider handling empty string totals more consistently.The logic for setting the total column converts zero values to empty strings, but non-zero values remain as numbers. This may cause type inconsistencies when used elsewhere.
for (const { name, offset, total } of this.things) { const row = new Array(columns).fill('') row[offset - 1] = name - row[columns - 1] = total > 0 ? total : '' + row[columns - 1] = total > 0 ? total.toString() : '' result.push(row) }src/client/utils.ts (1)
1-1: Use a regular expression literal instead ofRegExpconstructor.According to the static analysis hint, leveraging a literal syntax often simplifies your code and reduces escaping needs. Here's a suggested refactor:
-const githubRegex = new RegExp(`github.com/(?<owner>[^/]+)\\/(?<repo>[^/]+)\\/?$`) +const githubRegex = /github\.com\/(?<owner>[^/]+)\/(?<repo>[^/]+)\/?$/🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: Use a regular expression literal instead of the RegExp constructor.
Regular expression literals avoid some escaping required in a string literal, and are easier to analyze statically.
Safe fix: Use a literal notation instead.(lint/complexity/useRegexLiterals)
src/index.ts (1)
43-52: Optional enhancement for user safety.Consider adding a secondary confirmation or an undo step in
cleanThingsto prevent accidental irreversible deletions.src/parser/parser.ts (1)
15-31: Consider capturing headings with more context.
extractRawSectionis elegantly designed. However, if users include same-level subheadings in the targeted section, it could terminate early. A more robust approach for multi-level content might require deeper parsing.src/client/github.ts (2)
99-110: Wrap potential JSON parse errors.Consider a try/catch block to consistently handle fetch or parse failures.
112-124: Optionally handle parse failures.
fetchCommitalso relies on parseable JSON. Handling unexpected responses could prevent runtime errors.src/handlers/handlers.ts (4)
7-8: Avoid magic number fordefaultHeight.
Consider storingdefaultHeightin a shared configuration or explaining its purpose in a comment to improve clarity.
42-70: Add error handling for empty table of contents.
Ifparser.extractTableOfContents(readme)returns an empty array, the function will still proceed and may cause unexpected behavior. Consider aborting with a clear error message or a fallback note on the sheet in that scenario.
72-138: Reduce function complexity.
loadSectionperforms multiple steps: reading metadata, parsing content, creating a sheet, iterating items, and appending rows with error handling. Consider extracting some of these tasks into smaller functions for improved readability and testability.
140-145: Confirm bulk sheet deletion.
cleanThingsdeletes every sheet except the primary sheet. Verify users won’t accidentally lose important data. You might prompt users for confirmation in production scenarios.src/handlers/thing.ts (2)
3-24: Use a typed interface for readability.
Defining an interface or type for all possible fields can help ensure consistency and readability, especially as properties expand.
129-148: Ensure consistent ordering of object fields.
Relying onObject.values()andObject.keys()for the column order can be brittle if properties are rearranged later. Consider using a defined property array or a stable ordering approach for predictable columns.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (26)
.clasp.json.example(1 hunks).env(1 hunks).github/workflows/ci.yml(1 hunks).gitignore(1 hunks).prettierignore(1 hunks).prettierrc(1 hunks)README.md(1 hunks)appsscript.json(1 hunks)env.d.ts(1 hunks)eslint.config.js(1 hunks)package.json(1 hunks)src/client/github.ts(1 hunks)src/client/utils.ts(1 hunks)src/extra.ts(1 hunks)src/handlers/handlers.ts(1 hunks)src/handlers/thing.ts(1 hunks)src/handlers/toc.ts(1 hunks)src/handlers/urls.ts(1 hunks)src/index.ts(1 hunks)src/parser/parser.ts(1 hunks)src/vite-env.d.ts(1 hunks)tests/client/utils.test.ts(1 hunks)tests/parser/__mocks__/dummy.md(1 hunks)tests/parser/parser.test.ts(1 hunks)tsconfig.json(1 hunks)vite.config.ts(1 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[uncategorized] ~59-~59: You might be missing the article “the” here.
Context: ... ``` 3. Add the GitHub token to .env.local file 5. (Optional) Integr...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~78-~78: You might be missing the article “the” here.
Context: ...t 6. Deploy the project with following command: shell npm run deploy...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
tests/parser/__mocks__/dummy.md
[locale-violation] ~29-~29: Possible error ortogràfic. Aquesta forma verbal no es correspon amb la varietat seleccionada (central).
Context: .../> Per vel "Nobis si Dominium" urna ex dis est-etiam haeres mi maiores hic ille. ...
(EXIGEIX_VERBS_CENTRAL)
[grammar] ~29-~29: Hi falta algun element.
Context: ... Dominium" urna ex dis est-etiam haeres mi maiores hic ille. ## Header 1-2 ## He...
(MI)
[typographical] ~46-~46: Símbol sense parella: sembla que falta «[».
Context: ...editis--> ## Header 1-4 - [lucern-nisl](iusto://merita.non/moliri/sint/enim/ant...
(UNPAIRED_BRACKETS)
[typographical] ~47-~47: Símbol sense parella: sembla que falta «[».
Context: ...o Dicta Sint Pretium. - [sapien-antistes](dicta://lingua.quo/recordatio/pacifice/...
(UNPAIRED_BRACKETS)
[typographical] ~48-~48: Símbol sense parella: sembla que falta «[».
Context: ...ssilATE directe. - [specie-zelabant-sint](animi://aptent.quo/classica-wisi/orator...
(UNPAIRED_BRACKETS)
[typographical] ~54-~54: Símbol sense parella: sembla que falta «[».
Context: ... 1-4-1-1 - [nisl-iaculis-et-sit-regalia](ullam://tortor.nam/semente/typi-ultimae...
(UNPAIRED_BRACKETS)
[locale-violation] ~55-~55: Possible error ortogràfic. Aquesta forma verbal no es correspon amb la varietat seleccionada (central).
Context: ..., Rerum + ille-notare. - [lius-ministri-duis-vocatio](liber://possit.sed/regulantur/...
(EXIGEIX_VERBS_CENTRAL)
[typographical] ~55-~55: Símbol sense parella: sembla que falta «[».
Context: ...le-notare. - [lius-ministri-duis-vocatio](liber://possit.sed/regulantur/nisi-irri...
(UNPAIRED_BRACKETS)
[typographical] ~59-~59: Símbol sense parella: sembla que falta «[».
Context: ...lfstum. #### Header 1-4-1-2 - Videtur - Tri...
(UNPAIRED_BRACKETS)
[typographical] ~60-~60: Símbol sense parella: sembla que falta «[».
Context: ...ia potentia. - [odit-mi-virtutis-pubtico](minim://merita.hac/Fruges/typi-ut-abstu...
(UNPAIRED_BRACKETS)
[typographical] ~61-~61: Símbol sense parella: sembla que falta «[».
Context: ...dum.te eOs clari + EU. - [nam-esse-mazim](optio://debila.nec/arenam/quo-quas-culp...
(UNPAIRED_BRACKETS)
[typographical] ~65-~65: Símbol sense parella: sembla que falta «[».
Context: ...r 1-4-1-3 - [nisl-quoS-utriuque-placida](optio://patria.sem/disentitur/sint-quaM...
(UNPAIRED_BRACKETS)
[typographical] ~69-~69: Símbol sense parella: sembla que falta «[».
Context: ...c ContRarium. ### Header 1-4-2 - NuNc - mi...
(UNPAIRED_BRACKETS)
[grammar] ~69-~69: Hi falta algun element.
Context: ...c](nulla://aptent.nam/CrAbRones/MoDo) - mi a saepius tiomine dynamicus nam dicit e...
(MI)
[grammar] ~69-~69: Possible error ortogràfic.
Context: ...micus nam dicit ea Sint 9 + Sem 7 + ConsEntire + Louor (qui lius stipula adiurando)....
(APOSTROF_ACCENT)
[typographical] ~73-~73: Símbol sense parella: sembla que falta «[».
Context: ...4-2-1 - [wisi-uidem-maiestatem-stataria](animi://guttae.sem/Exuere/nisi-magni-de...
(UNPAIRED_BRACKETS)
[typographical] ~83-~83: Símbol sense parella: sembla que falta «[».
Context: ...Header 1-5-1-1 - [@martii/domini-sensim](zzril://florem.hac/laesit/wisi/sunt/sun...
(UNPAIRED_BRACKETS)
[typographical] ~84-~84: Símbol sense parella: sembla que falta «[».
Context: ...rcam decessu scomata. - [nisi-fronte-nam](minim://minaci.sem/zzril/wisi-platea-eo...
(UNPAIRED_BRACKETS)
[typographical] ~85-~85: Símbol sense parella: sembla que falta «[».
Context: ...Eros-tractu CUM. - [odit-capiat-impiorum](totam://dantis.per/magister/ullo-pungit...
(UNPAIRED_BRACKETS)
[locale-violation] ~85-~85: Possible error ortogràfic. Aquesta forma verbal no es correspon amb la varietat seleccionada (central).
Context: ...is.per/magister/ullo-pungit-potiorue) - Ullam NAM ingeminabit. - [ullo-victum-modo](l...
(EXIGEIX_VERBS_CENTRAL)
[typographical] ~86-~86: Símbol sense parella: sembla que falta «[».
Context: ...lam NAM ingeminabit. - [ullo-victum-modo](liber://lectus.mus/eum-ad/nisi-victor-q...
(UNPAIRED_BRACKETS)
[typographical] ~90-~90: Símbol sense parella: sembla que falta «[».
Context: ... #### Header 1-5-1-2 - [dominant-mazim](liber://motivo.eos/vitae/angulari-culpa...
(UNPAIRED_BRACKETS)
[grammar] ~90-~90: S’accentua si és del v. ‘tenir’.
Context: ...itae/angulari-culpa) - Versus benevolam te fames se discernere. - [typi-detrimento...
(TE)
[typographical] ~91-~91: Símbol sense parella: sembla que falta «[».
Context: ... fames se discernere. - [typi-detrimento](dicta://secuti.eum/JonasKruckenberg/ull...
(UNPAIRED_BRACKETS)
[locale-violation] ~103-~103: Possible error ortogràfic. Aquesta forma verbal no es correspon amb la varietat seleccionada (central).
Context: ...lorem est modo motivo. ![aD] dui Eos 8 duis, ![mE] hic Hac 4 orci, eum ![pER] est a...
(EXIGEIX_VERBS_CENTRAL)
[typographical] ~125-~125: Símbol sense parella: sembla que falta «[».
Context: ... Header 1-5-3-1 - [@tation/nativo-quasi](iusto://merita.hac/mattis/iste-hostis-m...
(UNPAIRED_BRACKETS)
[typographical] ~126-~126: Símbol sense parella: sembla que falta «[».
Context: ... quam Lacus. - [@futuri/vergit-dolor-nam](alias://dantis.rem/stilum/nisl-sancli-u...
(UNPAIRED_BRACKETS)
🪛 markdownlint-cli2 (0.17.2)
README.md
65-65: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
tests/parser/__mocks__/dummy.md
97-97: Link and image reference definitions should be needed
Unused link or image reference definition: "nam"
(MD053, link-image-reference-definitions)
98-98: Link and image reference definitions should be needed
Unused link or image reference definition: "ab"
(MD053, link-image-reference-definitions)
107-107: Link fragments should be valid
null
(MD051, link-fragments)
🪛 Biome (1.9.4)
src/client/utils.ts
[error] 1-1: Use a regular expression literal instead of the RegExp constructor.
Regular expression literals avoid some escaping required in a string literal, and are easier to analyze statically.
Safe fix: Use a literal notation instead.
(lint/complexity/useRegexLiterals)
🔇 Additional comments (41)
.prettierignore (1)
1-5: Configuration looks appropriate!The Prettier ignore list correctly excludes build artifacts (dist/) and configuration files that shouldn't be formatted. This ensures that configuration files maintain their specific formatting requirements and generated code remains untouched.
appsscript.json (1)
1-6: Google Apps Script configuration follows best practices.This configuration is well-structured for a Google Apps Script project:
- Using UTC timezone ensures consistent date/time handling
- STACKDRIVER for exception logging is the recommended approach
- V8 runtime provides access to modern JavaScript features
eslint.config.js (1)
1-17: ESLint configuration uses modern flat config format.The configuration properly integrates TypeScript ESLint with recommended rule sets, which is a good foundation for code quality.
.clasp.json.example (1)
1-6: Template provides a good starting point for CLASP configuration.The example configuration includes all necessary fields for Google Apps Script development with local TypeScript tooling. The rootDir pointing to "./dist" aligns with the TypeScript compilation output directory.
.gitignore (1)
1-20: Well-structured .gitignore file with appropriate entries.The .gitignore file includes all the necessary patterns for a Node.js project with Google Apps Script integration:
- Node.js specific ignores (node_modules, dist)
- Project-specific files (*.local, .clasp.json)
- Log files
- Editor/IDE files with a good optimization for VS Code (keeping extensions.json)
This follows best practices and will help keep the repository clean.
env.d.ts (1)
1-1: Appropriate TypeScript reference for Google Apps Script.The triple-slash reference directive correctly brings in type definitions from the
google-apps-scriptpackage, enabling proper TypeScript type checking and IntelliSense support for Google Apps Script APIs..prettierrc (1)
1-7: Well-structured Prettier configurationThe configuration establishes a clean, consistent code style with appropriate settings for the project. Using the organize-imports plugin is particularly helpful for maintaining organized import statements.
src/vite-env.d.ts (1)
1-10: Well-structured TypeScript declarations for environment variablesThe type declarations correctly extend Vite's type system to provide type safety for the environment variables used in the application. Using readonly modifiers is good practice to prevent accidental modification.
vite.config.ts (1)
31-36: Good use of path aliasesThe path alias configuration (
@pointing to./src) is a good practice that simplifies imports and improves code readability throughout the project.tests/client/utils.test.ts (1)
1-35: Well-structured and comprehensive test suite forparseLinkHeader.This test file provides thorough coverage of GitHub API pagination link header parsing scenarios. The parametrized approach with
describe.eachis efficient, and the test cases cover the various combinations of pagination links (next, prev, first, last) that might appear in real API responses.I appreciate the clear interface definition for test cases and the separation of test data from test logic, which makes the tests more maintainable.
package.json (1)
1-30: Configuration looks appropriate for the project requirements.The package.json is well-configured for a TypeScript project that integrates with Google Apps Script. The build and deploy scripts correctly handle the compilation and deployment flow using clasp, which aligns with the CI/CD objectives mentioned in the PR summary.
Dependencies are appropriate for the project's needs, with all development tools being recent versions. The single runtime dependency on "escape-string-regexp" is reasonable for string manipulation needs when parsing repository data.
tests/parser/parser.test.ts (1)
1-184: Comprehensive test coverage for the markdown parser functionality.The test suite thoroughly covers both the
extractThingsandextractTableOfContentsfunctions, which are core to the Table of Contents parser mentioned in the PR objectives. The tests are well-structured with:
- Clear test case definitions
- Comprehensive coverage of different header structures and nesting levels
- Detailed expected output validation
The use of a mock markdown file provides realistic test data, ensuring the parser can handle actual markdown content effectively.
tests/parser/__mocks__/dummy.md (1)
1-127: This mock file provides a good test case structure for the parser.The dummy markdown file includes a variety of headers (H1-H5), formatted links, lists, and custom HTML-like elements which should thoroughly test the parser's capabilities to extract table of contents and other elements.
🧰 Tools
🪛 LanguageTool
[locale-violation] ~29-~29: Possible error ortogràfic. Aquesta forma verbal no es correspon amb la varietat seleccionada (central).
Context: .../> Per vel "Nobis si Dominium" urna ex dis est-etiam haeres mi maiores hic ille. ...(EXIGEIX_VERBS_CENTRAL)
[grammar] ~29-~29: Hi falta algun element.
Context: ... Dominium" urna ex dis est-etiam haeres mi maiores hic ille. ## Header 1-2 ## He...(MI)
[typographical] ~46-~46: Símbol sense parella: sembla que falta «[».
Context: ...editis--> ## Header 1-4 - [lucern-nisl](iusto://merita.non/moliri/sint/enim/ant...(UNPAIRED_BRACKETS)
[typographical] ~47-~47: Símbol sense parella: sembla que falta «[».
Context: ...o Dicta Sint Pretium. - [sapien-antistes](dicta://lingua.quo/recordatio/pacifice/...(UNPAIRED_BRACKETS)
[typographical] ~48-~48: Símbol sense parella: sembla que falta «[».
Context: ...ssilATE directe. - [specie-zelabant-sint](animi://aptent.quo/classica-wisi/orator...(UNPAIRED_BRACKETS)
[typographical] ~54-~54: Símbol sense parella: sembla que falta «[».
Context: ... 1-4-1-1 - [nisl-iaculis-et-sit-regalia](ullam://tortor.nam/semente/typi-ultimae...(UNPAIRED_BRACKETS)
[locale-violation] ~55-~55: Possible error ortogràfic. Aquesta forma verbal no es correspon amb la varietat seleccionada (central).
Context: ..., Rerum + ille-notare. - [lius-ministri-duis-vocatio](liber://possit.sed/regulantur/...(EXIGEIX_VERBS_CENTRAL)
[typographical] ~55-~55: Símbol sense parella: sembla que falta «[».
Context: ...le-notare. - [lius-ministri-duis-vocatio](liber://possit.sed/regulantur/nisi-irri...(UNPAIRED_BRACKETS)
[typographical] ~59-~59: Símbol sense parella: sembla que falta «[».
Context: ...lfstum. #### Header 1-4-1-2 - Videtur - Tri...(UNPAIRED_BRACKETS)
[typographical] ~60-~60: Símbol sense parella: sembla que falta «[».
Context: ...ia potentia. - [odit-mi-virtutis-pubtico](minim://merita.hac/Fruges/typi-ut-abstu...(UNPAIRED_BRACKETS)
[typographical] ~61-~61: Símbol sense parella: sembla que falta «[».
Context: ...dum.te eOs clari + EU. - [nam-esse-mazim](optio://debila.nec/arenam/quo-quas-culp...(UNPAIRED_BRACKETS)
[typographical] ~65-~65: Símbol sense parella: sembla que falta «[».
Context: ...r 1-4-1-3 - [nisl-quoS-utriuque-placida](optio://patria.sem/disentitur/sint-quaM...(UNPAIRED_BRACKETS)
[typographical] ~69-~69: Símbol sense parella: sembla que falta «[».
Context: ...c ContRarium. ### Header 1-4-2 - NuNc - mi...(UNPAIRED_BRACKETS)
[grammar] ~69-~69: Hi falta algun element.
Context: ...c](nulla://aptent.nam/CrAbRones/MoDo) - mi a saepius tiomine dynamicus nam dicit e...(MI)
[grammar] ~69-~69: Possible error ortogràfic.
Context: ...micus nam dicit eaSint 9+Sem 7+ConsEntire+Louor(qui lius stipula adiurando)....(APOSTROF_ACCENT)
[typographical] ~73-~73: Símbol sense parella: sembla que falta «[».
Context: ...4-2-1 - [wisi-uidem-maiestatem-stataria](animi://guttae.sem/Exuere/nisi-magni-de...(UNPAIRED_BRACKETS)
[typographical] ~83-~83: Símbol sense parella: sembla que falta «[».
Context: ...Header 1-5-1-1 - [@martii/domini-sensim](zzril://florem.hac/laesit/wisi/sunt/sun...(UNPAIRED_BRACKETS)
[typographical] ~84-~84: Símbol sense parella: sembla que falta «[».
Context: ...rcam decessu scomata. - [nisi-fronte-nam](minim://minaci.sem/zzril/wisi-platea-eo...(UNPAIRED_BRACKETS)
[typographical] ~85-~85: Símbol sense parella: sembla que falta «[».
Context: ...Eros-tractu CUM. - [odit-capiat-impiorum](totam://dantis.per/magister/ullo-pungit...(UNPAIRED_BRACKETS)
[locale-violation] ~85-~85: Possible error ortogràfic. Aquesta forma verbal no es correspon amb la varietat seleccionada (central).
Context: ...is.per/magister/ullo-pungit-potiorue) - Ullam NAM ingeminabit. - [ullo-victum-modo](l...(EXIGEIX_VERBS_CENTRAL)
[typographical] ~86-~86: Símbol sense parella: sembla que falta «[».
Context: ...lam NAM ingeminabit. - [ullo-victum-modo](liber://lectus.mus/eum-ad/nisi-victor-q...(UNPAIRED_BRACKETS)
[typographical] ~90-~90: Símbol sense parella: sembla que falta «[».
Context: ... #### Header 1-5-1-2 - [dominant-mazim](liber://motivo.eos/vitae/angulari-culpa...(UNPAIRED_BRACKETS)
[grammar] ~90-~90: S’accentua si és del v. ‘tenir’.
Context: ...itae/angulari-culpa) - Versus benevolam te fames se discernere. - [typi-detrimento...(TE)
[typographical] ~91-~91: Símbol sense parella: sembla que falta «[».
Context: ... fames se discernere. - [typi-detrimento](dicta://secuti.eum/JonasKruckenberg/ull...(UNPAIRED_BRACKETS)
[locale-violation] ~103-~103: Possible error ortogràfic. Aquesta forma verbal no es correspon amb la varietat seleccionada (central).
Context: ...lorem est modo motivo. ![aD] dui Eos 8 duis, ![mE] hic Hac 4 orci, eum ![pER] est a...(EXIGEIX_VERBS_CENTRAL)
[typographical] ~125-~125: Símbol sense parella: sembla que falta «[».
Context: ... Header 1-5-3-1 - [@tation/nativo-quasi](iusto://merita.hac/mattis/iste-hostis-m...(UNPAIRED_BRACKETS)
[typographical] ~126-~126: Símbol sense parella: sembla que falta «[».
Context: ... quam Lacus. - [@futuri/vergit-dolor-nam](alias://dantis.rem/stilum/nisl-sancli-u...(UNPAIRED_BRACKETS)
🪛 markdownlint-cli2 (0.17.2)
97-97: Link and image reference definitions should be needed
Unused link or image reference definition: "nam"(MD053, link-image-reference-definitions)
98-98: Link and image reference definitions should be needed
Unused link or image reference definition: "ab"(MD053, link-image-reference-definitions)
107-107: Link fragments should be valid
null(MD051, link-fragments)
.github/workflows/ci.yml (2)
3-8: CI/CD trigger configuration looks good.The workflow is correctly set up to run on both pushes to main and all pull requests.
32-50: Test job is well structured.The test job correctly depends on the lint job and follows the same pattern for checkout, setup, and dependency installation.
tsconfig.json (2)
1-28: TypeScript configuration is well-structured for a Google Apps Script project.The configuration includes the necessary type definitions for Google Apps Script (
@types/google-apps-script), along with appropriate module settings and strict type checking.
25-25:Details
✅ Verification successful
Verify the need for
@types/parse-link-header.This type definition is included but it's not immediately clear where it's used in the project. Ensure it's actually needed.
🏁 Script executed:
#!/bin/bash # Check if parse-link-header is used in the codebase grep -r "parse-link-header" --include="*.ts" . grep -r "parseLinkHeader" --include="*.ts" .Length of output: 652
Clarification on
@types/parse-link-header: Dependency is ValidThe grep output confirms that the type definitions for
parse-link-headerare indeed needed. Specifically:
parseLinkHeaderis called in src/client/github.ts.- The utility is defined and used in src/client/utils.ts.
- Usage is also verified in the test suite tests/client/utils.test.ts.
No changes are required.
src/client/utils.ts (3)
3-5: Looks good!This
isSupportedUrlfunction cleanly checks if the URL matches your GitHub regex pattern.
7-16: Consider verifying usage coverage in tests.Your
parseOwnerRepofunction properly throws an error for invalid GitHub URLs. Verify that all functions callingparseOwnerRepohandle or catch this error to avoid unhandled exceptions.
22-34: Implementation is straightforward.
parseLinkHeaderreads clearly. Good job on returning an empty object for invalid or empty headers. This function is well-structured.src/index.ts (3)
3-10: Menu creation looks good.Google Apps Script’s
onOpenusage is correct and the menu items integrate seamlessly.
12-26: Validate user input before loading content.You prompt the user for a repository link but do not validate it locally (e.g., using
isSupportedUrl) before passing it tohandlers.loadTopOfContent. Consider verifying the URL to handle malformed links gracefully.
28-41: No functional issues found.Your
loadThingfunction checks the active sheet and selected value properly before loading the section.src/parser/parser.ts (5)
3-8: Interfaces look fine.
Thingdefinition is self-explanatory and provides good clarity for name, URL, and description.
9-13: Well-structured interface.
TableOfContentsis concise and effectively captures the section’s metadata.
33-33: Regex for item matching is clear.The usage of a named capturing group ensures readability.
35-51: Functionality is good.
extractThingsgracefully handles the scenario where a section doesn’t exist, returning an empty array. The map & filter steps are well-structured.
53-75: Nice approach for generating a table of contents.Your sequence of mapping and filtering is straightforward, ensuring non-null lines are processed.
src/client/github.ts (12)
3-23: Repository interface is comprehensive.Captures the essential fields for a GitHub repository.
25-27: Commit interface is succinct and clear.No issues here.
29-31: Tag interface recognized.No additional comments.
33-35: Release interface recognized.No additional comments.
37-40: IssueStats interface is fine.Straightforward representation of open/closed data.
42-54: Comprehensive Metadata interface.This is carefully built to hold various pieces of repository data.
56-61: Verify environment variable availability.
import.meta.env.VITE_GITHUB_TOKENmust be set appropriately at runtime or build time. Ensure that your CI/CD pipeline securely manages this token.
63-97: fetchRepository handles errors gracefully.Your try/catch block logs an error and rethrows. This is good for debugging.
126-153: Efficient approach.
fetchLastAndTotalneatly grabs the first item and calculates total items from link headers.
155-164: fetchTotal usage is straightforward.No concerns here.
166-185: Issue/pr separation logic is correct.Subtracting pull requests from issues is a valid approach given GitHub’s combined “issues” endpoint.
187-212: metaByUrl is well organized.Combines all relevant repository data into one cohesive metadata object. Good job!
src/handlers/thing.ts (1)
164-170: Verify the key-value pairing remains synchronized.
build()andheaders()both rely onthis.doBuild()but generate arrays by iterating over the object keys and values. Any change indoBuild()could affect both. Confirm that all consumers of this data structure can handle potential column reordering.
There was a problem hiding this comment.
Pull Request Overview
This PR delivers the first MVP build of the project, implementing a custom Google Sheets menu to load and display GitHub repository metadata, alongside functionality for parsing README tables of contents and handling GitHub API integrations. Key changes include:
- New documentation and setup instructions in README.md.
- Implementation of GitHub API fetch functions, URL parsing utilities, and parser functions.
- New CI/CD pipeline, linting, and testing configurations.
Reviewed Changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| README.md | Added project overview, usage instructions, and setup guidelines. |
| eslint.config.js | Introduced basic ESLint configuration with TypeScript settings. |
| .github/workflows/ci.yml | Added CI/CD pipeline using GitHub Actions. |
| src/index.ts | Defined Apps Script functions for menu actions in Google Sheets. |
| src/client/utils.ts | New utility functions for URL validation and parsing. |
| src/parser/parser.ts | Added functions to extract table of contents and things from a README. |
| src/handlers/handlers.ts | Implemented handlers for loading and cleaning sheet data. |
| src/handlers/thing.ts | Introduced a builder for formatting repository metadata into rows. |
| tests/client/utils.test.ts | Added tests for link header parsing. |
| src/handlers/toc.ts | Added a builder for constructing table of contents data. |
| src/handlers/urls.ts | Integrated URL generation for stars explorer links. |
| src/client/github.ts | Added functions for fetching repository and commit data via GitHub API. |
| src/extra.ts | Extra snippet for running section loading (Apps Script context). |
| tests/parser/mocks/dummy.md | Provided a dummy markdown file to support parser tests. |
Comments suppressed due to low confidence (2)
README.md:5
- [nitpick] The sentence is awkward; consider revising to '...track the project's dynamics and ultimately make an informed decision about which solution to pick.'
**awesome-things** helps you organize all the options in one table, compare the numbers of stars, open/closed issues/PRs, track the project's dynamic and ultimately make an informed which solution to pick.
src/client/github.ts:122
- The property name 'commitedAt' appears to be misspelled. Consider renaming it to 'committedAt' for clarity and correctness.
commitedAt: data.commit.committer.date,
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
This PR implements the first MVP build by introducing a Table-of-Contents parser, GitHub repository metadata collection via the GitHub API, and integration with Google Sheets, alongside establishing a CI/CD pipeline and comprehensive usage documentation.
- Added custom Google Sheets menu and functions that load and clean content.
- Implemented GitHub integration to fetch and display repository metadata.
- Configured CI/CD and provided detailed setup instructions in the README.
Reviewed Changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| eslint.config.js | Added ESLint configuration with TypeScript and recommended rules. |
| README.md | New documentation detailing project setup, usage, and prerequisites. |
| .github/workflows/ci.yml | CI/CD pipeline configuration for linting, testing, and building the project. |
| src/index.ts | Main entry point establishing custom menu options in Google Sheets. |
| src/handlers/toc.ts | Implementation of a Table-of-Contents builder. |
| src/parser/parser.ts | Parser functions to extract repository sections and generate structured data. |
| src/client/utils.ts | Utility functions for GitHub URL parsing and link header extraction. |
| src/handlers/handlers.ts | Integration logic for fetching GitHub metadata, populating sheets, and error handling. |
| src/handlers/thing.ts | Builder class for formatting fetched GitHub metadata into spreadsheet rows. |
| tests/client/utils.test.ts | Unit tests for URL and header parsing functionality. |
| src/handlers/urls.ts | Helper function generating URL for the Stars Explorer. |
| src/client/github.ts | GitHub API calls for fetching repository details, commits, tags, and releases. |
| src/extra.ts | Entry point for triggering content load in a live spreadsheet. |
| tests/parser/mocks/dummy.md | Mock markdown file for testing the parser's behavior. |
Comments suppressed due to low confidence (3)
README.md:3
- [nitpick] Consider rephrasing for clarity, e.g. 'Have you ever faced the challenge of choosing the right solution among many alternatives?'
Have you faced ever with the challenge of choosing the right solutions among many alternatives?
README.md:5
- [nitpick] Revise the phrase 'make an informed which solution to pick' for clarity, for example: 'make an informed decision on which solution to pick'.
**awesome-things** helps you organize all the options in one table, compare the numbers of stars, open/closed issues/PRs, track the project's dynamic and ultimately make an informed which solution to pick.
src/client/github.ts:122
- Typo detected in the property name 'commitedAt'; consider renaming it to 'committedAt' for consistency.
commitedAt: data.commit.committer.date,
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (7)
README.md (5)
3-3: Improve the wording in this sentence.The phrase "Have you faced ever with the challenge" uses an unnatural word order.
-Have you faced ever with the challenge of choosing the right solutions among many alternatives? +Have you ever faced the challenge of choosing the right solutions among many alternatives?🧰 Tools
🪛 LanguageTool
[uncategorized] ~3-~3: A different word order might sound more natural.
Context: # awesome-things Have you faced ever with the challenge of choosing the righ...(AI_HYDRA_LEO_WORD_ORDER)
5-5: Add a missing word for clarity.The sentence is missing the word "decision" after "informed".
-**awesome-things** helps you organize all the options in one table, compare the numbers of stars, open/closed issues/PRs, track the project's dynamic and ultimately make an informed which solution to pick. +**awesome-things** helps you organize all the options in one table, compare the numbers of stars, open/closed issues/PRs, track the project's dynamic and ultimately make an informed decision which solution to pick.
59-59: Add missing article.Add the definite article "the" before ".env.local file".
- 3. Add the GitHub token to `.env.local` file + 3. Add the GitHub token to the `.env.local` file🧰 Tools
🪛 LanguageTool
[uncategorized] ~59-~59: You might be missing the article “the” here.
Context: ... ``` 3. Add the GitHub token to.env.localfile 5. (Optional) Integr...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
65-67: Add language specifier to fenced code block.According to markdown best practices, fenced code blocks should specify a language.
- ``` + ```env VITE_STARS_EXPLORER_URL=127.0.0.1:8080 ```🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
65-65: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
78-78: Add missing article.Add the definite article "the" before "following command".
-6. Deploy the project with following command: +6. Deploy the project with the following command:🧰 Tools
🪛 LanguageTool
[uncategorized] ~78-~78: You might be missing the article “the” here.
Context: ...t6. Deploy the project with following command:shell npm run deploy...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
src/handlers/toc.ts (1)
11-13: Consider returning a header row when no data is available.Currently, when
this.thingsis empty, the function returns an empty array. Returning at least a header row would provide a consistent structure and avoid potential issues with downstream code expecting headers.if (this.things.length === 0) { - return [] + const header = ['Header 1', 'Total'] + return [header] }src/handlers/handlers.ts (1)
56-58: Add user feedback when no rows are built.When the table of contents is empty (
rows.length === 0), the function silently returns without providing any feedback. Consider adding a user-friendly message to the sheet.if (rows.length === 0) { + sheet.getRange(2, 1).setValue("No table of contents found in the README. Please check the repository URL.") return }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
README.md(1 hunks)src/handlers/handlers.ts(1 hunks)src/handlers/toc.ts(1 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[uncategorized] ~3-~3: A different word order might sound more natural.
Context: # awesome-things Have you faced ever with the challenge of choosing the righ...
(AI_HYDRA_LEO_WORD_ORDER)
[uncategorized] ~59-~59: You might be missing the article “the” here.
Context: ... ``` 3. Add the GitHub token to .env.local file 5. (Optional) Integr...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~78-~78: You might be missing the article “the” here.
Context: ...t 6. Deploy the project with following command: shell npm run deploy...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
🪛 markdownlint-cli2 (0.17.2)
README.md
65-65: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
🔇 Additional comments (6)
README.md (1)
53-53: Fix typo in setup instruction.There's a typo in the word "Create".
- 2. Crate a `.env.local` + 2. Create a `.env.local`src/handlers/toc.ts (2)
18-20: Code looks good!The header generation logic is clean and well-structured. It dynamically creates headers based on the maximum offset value.
22-27: Handling of zero or negative totals looks good.The code correctly handles cases where
totalis zero or negative by displaying an empty string. This helps maintain a clean display in the spreadsheet.src/handlers/handlers.ts (3)
10-31: Consider revisiting the sheet position logic.The insertion index calculation using
Math.max(2, index)enforces a minimum position of 2, which may not be intuitive. This could lead to unexpected sheet ordering, especially in spreadsheets with fewer than 2 sheets.#!/bin/bash # Check if there are any potential issues with sheet positioning in other parts of the codebase rg -A 3 -B 3 "insertSheet\(" --type ts
94-94: Verify if column width should be set to defaultHeight.Setting column width to
defaultHeight(21) seems unusual. Verify if this is intentional or if these should be different constants.Is this column intended to display small icons or avatars? If so, consider adding a comment explaining why this specific width was chosen.
98-137: Good error handling for GitHub API calls.The try-catch block for each repository is a good practice. It ensures that a failure with one repository doesn't prevent loading data for other repositories. The error is properly logged and displayed in the spreadsheet.
Summary by CodeRabbit
New Features
Documentation