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.

Sign up for GitHub

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

Fix navbar behaviour when <body> isn't on its own line; add tests for this #10

Closed
wants to merge 2 commits into from
Closed

Fix navbar behaviour when <body> isn't on its own line; add tests for this #10

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
aecd373
Add tests for navbar
penelopeysm Feb 1, 2025
fff84d1
Loosen regex
penelopeysm Feb 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions .github/workflows/test_navbar_script.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
# This workflow runs the insert_navbar script on several test HTML inputs,
# making sure that the output is as expected.

name: Test Navbar Script

on:
push:
branches:
- main
pull_request:

jobs:
test-html-diff:
runs-on: ubuntu-latest

steps:
- name: Checkout repository
uses: actions/checkout@v4

- name: Copy script to GHA scratch directory
run: cp DocsNav/scripts/insert_navbar.sh .github/workflows/test_navbar_script

- name: Compare all test inputs against out.html
working-directory: .github/workflows/test_navbar_script
run: |
# Make script executable
chmod +x ./insert_navbar.sh
# Run script on all files in `ins_html` directory
./insert_navbar.sh ./ins_html ./test_navbar.html
# Expected output
OUT="expected_out.html"
# Check if all files in `ins_html` match the expected output
for file in ins_html/*; do
if diff <(tr -d '[:space:]' < "$file") <(tr -d '[:space:]' < $OUT); then
Comment on lines +33 to +34
Copy link
Member Author

@penelopeysm penelopeysm Feb 1, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This is also not technically correct, because HTML is not totally whitespace-insignificant - for example tag names and attributes must be separated - but I suppose it's not really possible to test this without using a proper HTML parser, and for the limited test examples I reckon this should be fine.

echo "✅ $file matches $OUT (ignoring whitespace)"
else
echo "❌ $file differs from $OUT"
exit 1 # Fail the job if any file is different
fi
done
7 changes: 7 additions & 0 deletions .github/workflows/test_navbar_script/expected_out.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<html>
<body>
<p>This is a navbar!</p>
<h1>Test 1</h1>
<p>Test 1</p>
</body>
</html>
6 changes: 6 additions & 0 deletions .github/workflows/test_navbar_script/ins_html/test1_in.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<html>
<body>
<h1>Test 1</h1>
<p>Test 1</p>
</body>
</html>
4 changes: 4 additions & 0 deletions .github/workflows/test_navbar_script/ins_html/test2_in.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
<html>
<body><h1>Test 1</h1><p>Test 1</p>
</body>
</html>
5 changes: 5 additions & 0 deletions .github/workflows/test_navbar_script/ins_html/test3_in.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<html><body>
<h1>Test 1</h1>
<p>Test 1</p>
</body>
</html>
2 changes: 2 additions & 0 deletions .github/workflows/test_navbar_script/ins_html/test4_in.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
<html><body><h1>Test 1</h1><p>Test 1</p>
</body></html>
1 change: 1 addition & 0 deletions .github/workflows/test_navbar_script/test_navbar.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<p>This is a navbar!</p>
6 changes: 2 additions & 4 deletions DocsNav/scripts/insert_navbar.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,8 @@ find "$HTML_DIR" -type f -name "*.html" | while read -r file; do

# Insert the new navbar right after the first <body> tag using awk
awk -v nav="$NAVBAR_HTML" '
/<body>/ {
print $0
print nav
next
/<body[^>]*>/ {
sub(/(<body[^>]*>)/, "&\n" nav);
Comment on lines +86 to +87
Copy link
Member Author

@penelopeysm penelopeysm Feb 1, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

The new regex is more flexible in that it would picks up cases where the body tag has some attribute <body class="myclass">. I suppose it's still technically sort of fragile, in the sense that if the attribute contains > it will break. Happy to go either way on this although I think the new regex is a bit better in that the failure case is much less likely than the failure case for the old one.

}
{ print }
' "$file" > temp && mv temp "$file"
Expand Down