Fix theme from upstream #151
Conversation
|
Warning Review failedThe pull request is closed. WalkthroughThis update includes a variety of changes across multiple files within the Academia theme and related scripts for the Hugo static site generator. Key adjustments involve improvements to URL handling, resource retrieval logic, HTML structure, accessibility features, and project setup scripts. Notably, it also includes an upgrade of the Hugo version from 0.122.0 to 0.128.2 and introduces new functionality for handling content expansion and Microsoft Clarity tracking. Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 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 as PR comments)
Additionally, you can add CodeRabbit Configration 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: 6
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (13)
- themes/academia/layouts/authors/list.html (1 hunks)
- themes/academia/layouts/partials/docs_sidebar.html (1 hunks)
- themes/academia/layouts/partials/li_card.html (2 hunks)
- themes/academia/layouts/partials/li_list.html (1 hunks)
- themes/academia/layouts/partials/navbar.html (2 hunks)
- themes/academia/layouts/partials/page_links_div.html (1 hunks)
- themes/academia/layouts/partials/site_footer.html (2 hunks)
- themes/academia/layouts/partials/site_head.html (3 hunks)
- themes/academia/layouts/partials/widgets/about.html (1 hunks)
- themes/academia/layouts/partials/widgets/pages.html (1 hunks)
- themes/academia/layouts/shortcodes/expand.html (1 hunks)
- themes/academia/scripts/projectSetup.js (1 hunks)
- themes/academia/scripts/themeSetup.js (1 hunks)
Files skipped from review due to trivial changes (4)
- themes/academia/layouts/partials/li_card.html
- themes/academia/layouts/partials/widgets/about.html
- themes/academia/layouts/partials/widgets/pages.html
- themes/academia/layouts/shortcodes/expand.html
Additional context used
Biome
themes/academia/scripts/projectSetup.js
[error] 1-1: A Node.js builtin module should be imported with the node: protocol.
[error] 2-2: A Node.js builtin module should be imported with the node: protocol.
[error] 12-12: Change to an optional chain.
[error] 33-45: Prefer for...of instead of forEach.
[error] 58-69: Prefer for...of instead of forEach.
themes/academia/scripts/themeSetup.js
[error] 1-1: A Node.js builtin module should be imported with the node: protocol.
[error] 2-2: A Node.js builtin module should be imported with the node: protocol.
[error] 24-24: Change to an optional chain.
[error] 33-45: Prefer for...of instead of forEach.
[error] 61-77: Prefer for...of instead of forEach.
Additional comments not posted (10)
themes/academia/layouts/partials/page_links_div.html (1)
18-18: LGTM! Ensure that the newParams.doicondition is properly documented and tested.themes/academia/layouts/authors/list.html (1)
17-18: LGTM! Verify that the new URL extraction logic works correctly across different URL formats.Verification successful
The new URL extraction logic has been successfully verified across different URL formats, and all test cases passed.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the new URL extraction logic in different URL formats. # Test: Manually test with different URL formats. Expect: Correct extraction of usernames. echo "Manual testing required."Length of output: 58
Script:
#!/bin/bash # Description: Automate the verification of the new URL extraction logic in different URL formats. # Function to simulate the URL extraction logic extract_username() { local url=$1 local base_url="https://example.com/" local base_excluded=${url#"$base_url"} local username=$(echo $base_excluded | cut -d'/' -f2) echo $username } # Test cases urls=( "https://example.com/user/johndoe" "https://example.com/user/janedoe" "https://example.com/user/alexsmith" "https://example.com/user/maryjane" ) # Expected usernames expected_usernames=( "johndoe" "janedoe" "alexsmith" "maryjane" ) # Run tests for i in "${!urls[@]}"; do url=${urls[$i]} expected=${expected_usernames[$i]} result=$(extract_username "$url") if [ "$result" == "$expected" ]; then echo "PASS: $url -> $result" else echo "FAIL: $url -> $result (expected: $expected)" fi doneLength of output: 2230
themes/academia/layouts/partials/site_footer.html (1)
3-4: LGTM! Visually inspect the footer to ensure the alignment and external link handling are as expected.Also applies to: 17-34
themes/academia/layouts/partials/li_list.html (1)
21-21: LGTM! Check the responsiveness of the new layout on different devices to ensure it scales properly.themes/academia/layouts/partials/docs_sidebar.html (1)
4-7: LGTM! Verify that the new menu name extraction logic works correctly across different configurations.themes/academia/scripts/themeSetup.js (3)
4-8: ThecreateNewfolderfunction correctly creates a new directory. Good use of therecursiveoption.
10-14: ThedeleteFolderfunction safely checks for existence before deletion, which is a good practice.
48-85: ThesetupThemefunction is well-organized and effectively uses helper functions to manage theme setup.themes/academia/layouts/partials/navbar.html (1)
14-14: The change to backticks for thei18nfunction call in thearia-labelattribute is a good practice for consistency in Hugo templates.themes/academia/layouts/partials/site_head.html (1)
7-7: The addition of thetheme-namemeta tag, Microsoft Clarity script, andarticle:published_timemeta tag are well-implemented and enhance the functionality and SEO of the site.Also applies to: 121-129, 189-189
| @@ -0,0 +1,87 @@ | |||
| const fs = require("fs"); | |||
There was a problem hiding this comment.
Use the node: protocol for built-in module imports to ensure compatibility with newer Node.js versions.
- const fs = require("fs");
- const path = require("path");
+ const fs = require("node:fs");
+ const path = require("node:path");Also applies to: 2-2
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| const fs = require("fs"); | |
| const fs = require("node:fs"); | |
| const path = require("node:path"); |
Tools
Biome
[error] 1-1: A Node.js builtin module should be imported with the node: protocol.
| const iterateFilesAndFolders = (rootFolder, { destinationRoot }) => { | ||
| const directory = path.join(rootFolder); | ||
| const items = fs.readdirSync(directory, { withFileTypes: true }); | ||
| items.forEach((item) => { | ||
| if (item.isDirectory()) { | ||
| createNewfolder(destinationRoot, item.name); | ||
| iterateFilesAndFolders(path.join(directory, item.name), { | ||
| currentFolder: item.name, | ||
| destinationRoot: path.join(destinationRoot, item.name), | ||
| }); | ||
| } else { | ||
| const sourceFile = path.join(directory, item.name); | ||
| const destinationFile = path.join(destinationRoot, item.name); | ||
| fs.renameSync(sourceFile, destinationFile); | ||
| } | ||
| }); | ||
| }; |
There was a problem hiding this comment.
Replace forEach with for...of for better control over iteration, especially useful if asynchronous operations are needed in the future.
- items.forEach((item) => {
+ for (const item of items) {Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| const iterateFilesAndFolders = (rootFolder, { destinationRoot }) => { | |
| const directory = path.join(rootFolder); | |
| const items = fs.readdirSync(directory, { withFileTypes: true }); | |
| items.forEach((item) => { | |
| if (item.isDirectory()) { | |
| createNewfolder(destinationRoot, item.name); | |
| iterateFilesAndFolders(path.join(directory, item.name), { | |
| currentFolder: item.name, | |
| destinationRoot: path.join(destinationRoot, item.name), | |
| }); | |
| } else { | |
| const sourceFile = path.join(directory, item.name); | |
| const destinationFile = path.join(destinationRoot, item.name); | |
| fs.renameSync(sourceFile, destinationFile); | |
| } | |
| }); | |
| }; | |
| const iterateFilesAndFolders = (rootFolder, { destinationRoot }) => { | |
| const directory = path.join(rootFolder); | |
| const items = fs.readdirSync(directory, { withFileTypes: true }); | |
| for (const item of items) { | |
| if (item.isDirectory()) { | |
| createNewfolder(destinationRoot, item.name); | |
| iterateFilesAndFolders(path.join(directory, item.name), { | |
| currentFolder: item.name, | |
| destinationRoot: path.join(destinationRoot, item.name), | |
| }); | |
| } else { | |
| const sourceFile = path.join(directory, item.name); | |
| const destinationFile = path.join(destinationRoot, item.name); | |
| fs.renameSync(sourceFile, destinationFile); | |
| } | |
| } | |
| }; |
Tools
Biome
[error] 33-45: Prefer for...of instead of forEach.
| const getFolderName = (rootfolder) => { | ||
| const configPath = path.join( | ||
| rootfolder, | ||
| "exampleSite/config/_default/hugo.toml" | ||
| ); | ||
| const getConfig = fs.readFileSync(configPath, "utf8"); | ||
| const match = getConfig.match(/theme\s*=\s*\[?"([^"\]]+)"\]?/); | ||
| let selectedTheme = null; | ||
| if (match && match[1]) { | ||
| selectedTheme = match[1]; | ||
| } | ||
| return selectedTheme; | ||
| }; |
There was a problem hiding this comment.
Consider using optional chaining when accessing potentially undefined properties to enhance code robustness.
- if (match && match[1]) {
+ if (match?.[1]) {Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| const getFolderName = (rootfolder) => { | |
| const configPath = path.join( | |
| rootfolder, | |
| "exampleSite/config/_default/hugo.toml" | |
| ); | |
| const getConfig = fs.readFileSync(configPath, "utf8"); | |
| const match = getConfig.match(/theme\s*=\s*\[?"([^"\]]+)"\]?/); | |
| let selectedTheme = null; | |
| if (match && match[1]) { | |
| selectedTheme = match[1]; | |
| } | |
| return selectedTheme; | |
| }; | |
| const getFolderName = (rootfolder) => { | |
| const configPath = path.join( | |
| rootfolder, | |
| "exampleSite/config/_default/hugo.toml" | |
| ); | |
| const getConfig = fs.readFileSync(configPath, "utf8"); | |
| const match = getConfig.match(/theme\s*=\s*\[?"([^"\]]+)"\]?/); | |
| let selectedTheme = null; | |
| if (match?.[1]) { | |
| selectedTheme = match[1]; | |
| } | |
| return selectedTheme; | |
| }; |
Tools
Biome
[error] 24-24: Change to an optional chain.
| const fs = require("fs"); | ||
| const path = require("path"); |
There was a problem hiding this comment.
Use the node: protocol for importing Node.js built-in modules.
- const fs = require("fs");
- const path = require("path");
+ const fs = require("node:fs");
+ const path = require("node:path");Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| const fs = require("fs"); | |
| const path = require("path"); | |
| const fs = require("node:fs"); | |
| const path = require("node:path"); |
Tools
Biome
[error] 1-1: A Node.js builtin module should be imported with the node: protocol.
[error] 2-2: A Node.js builtin module should be imported with the node: protocol.
| const getConfig = fs.readFileSync(configPath, "utf8"); | ||
| const match = getConfig.match(/theme\s*=\s*\[?"([^"\]]+)"\]?/); | ||
| let selectedTheme = null; | ||
| if (match && match[1]) { |
There was a problem hiding this comment.
Use optional chaining to simplify the code.
- if (match && match[1]) {
+ if (match?.[1]) {Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| if (match && match[1]) { | |
| if (match?.[1]) { |
Tools
Biome
[error] 12-12: Change to an optional chain.
| items.forEach((item) => { | ||
| if (item.isDirectory()) { | ||
| createNewfolder(destinationRoot, item.name); | ||
| iterateFilesAndFolders(path.join(directory, item.name), { | ||
| currentFolder: item.name, | ||
| destinationRoot: path.join(destinationRoot, item.name), | ||
| }); | ||
| } else { | ||
| const sourceFile = path.join(directory, item.name); | ||
| const destinationFile = path.join(destinationRoot, item.name); | ||
| fs.renameSync(sourceFile, destinationFile); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Prefer for...of instead of forEach for better readability and performance.
- items.forEach((item) => {
+ for (const item of items) {Also applies to: 58-69
Committable suggestion was skipped due low confidence.
Tools
Biome
[error] 33-45: Prefer for...of instead of forEach.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- themes/academia/layouts/partials/comments.html (1 hunks)
- themes/academia/layouts/partials/page_metadata.html (1 hunks)
- themes/academia/layouts/partials/site_head.html (3 hunks)
- themes/academia/layouts/partials/site_js.html (1 hunks)
Files skipped from review due to trivial changes (1)
- themes/academia/layouts/partials/comments.html
Additional comments not posted (5)
themes/academia/layouts/partials/page_metadata.html (1)
52-52: Update reference to Disqus shortname.The reference to the Disqus shortname has been updated from
site.DisqusShortnametosite.Config.Services.Disqus.Shortname. Ensure that this change is consistent throughout the codebase.Verification successful
Update reference to Disqus shortname verified.
The reference to the Disqus shortname has been updated consistently from
site.DisqusShortnametosite.Config.Services.Disqus.Shortnameacross the codebase. The old reference is no longer present.
themes/academia/layouts/partials/site_js.htmlthemes/academia/layouts/partials/page_metadata.htmlthemes/academia/layouts/partials/comments.htmlScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the reference to Disqus shortname is updated consistently. # Test: Search for the old reference. Expect: No occurrences. rg --type yaml --type html --type js $'site.DisqusShortname' # Test: Search for the new reference. Expect: Only valid occurrences. rg --type yaml --type html --type js $'site.Config.Services.Disqus.Shortname'Length of output: 845
themes/academia/layouts/partials/site_js.html (1)
48-50: Update reference to Disqus shortname.The reference to the Disqus shortname has been updated from
site.DisqusShortnametosite.Config.Services.Disqus.Shortname. Ensure that this change is consistent throughout the codebase.themes/academia/layouts/partials/site_head.html (3)
7-7: Add meta tag for theme name.A new meta tag for the theme name has been added. This is a useful addition for identifying the theme in use.
120-129: Include Microsoft Clarity script conditionally.The Microsoft Clarity script is conditionally included if
site.Params.microsoft_clarityis set. This is a good practice to ensure the script is only loaded when necessary.
Line range hint
130-137:
Include Google Analytics script conditionally.The Google Analytics script is conditionally included if
site.Config.Services.GoogleAnalytics.IDis set. This is a good practice to ensure the script is only loaded when necessary.
Summary by CodeRabbit
New Features
Bug Fixes
Improvements
Chores
.gitignore.Refactors