Skip to content

make format-xml to reformat, make hook to set up pre-commit hook #28

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

make format-xml to reformat, make hook to set up pre-commit hook #28

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
18 changes: 16 additions & 2 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
export

SHELL = bash

# Language to build. Default: $(LANG)
LANG = en

Expand Down Expand Up @@ -33,8 +35,10 @@ help:
@echo ""
@echo " Targets"
@echo ""
@echo " deps Download DITA-OT dist"
@echo " build Build HTML"
@echo " deps Download DITA-OT dist"
@echo " build Build HTML"
@echo " format-xml Consistently indent XML"
@echo " hook Install git pre-commit hook"
@echo ""
@echo " Variables"
@echo ""
Expand Down Expand Up @@ -64,3 +68,13 @@ build:
--args.input.dir="$(REPODIR)" \
--propertyfile="$(DITA_PROPERTY_FILE)"
cp -r $(REPODIR)/resources/ $(GT_DOC_OUT)

# Consistently indent XML
format-xml:
find . -path ./$(notdir $(DITA_OT_DIR)) -prune -false -o -name '*.dita' -a -not \( -size 0 \) |while read i;do \
echo xmlstarlet fo $$i > $$i.formatted && mv $$i.formatted $$i; \
Comment on lines +74 to +75
Copy link

Choose a reason for hiding this comment

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

Forgive my being inquisitive, but what's -false -o for? IIUC, -prune always evaluates to true, then -false always to false, then -o is a disjunction, then comes -name. So IINM that together means the same as -prune -name '*.dita'.

For \( -size 0 \) there's also -empty BTW.

The shell's while loop could be rewritten as | xargs 'xmlstarlet fo {} | sponge {}' or | parallel -j 'xmlstarlet fo {} | sponge {}' BTW.

Copy link
Member Author

Choose a reason for hiding this comment

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

Inquisitiveness is appreciated here.

-prune -false -o is one of those snippets I have in muscle memory. It doesn't do the right thing without -false -o. I agree it should but it doesn't. find(1) is weird sometimes.

\( -size 0 \) == -empty thanks, I'll change it.

As for sponge/parallel: Thanks for the tip, sponge solves the redirection issue and your approach is probably more efficient, but the while loop is easier to debug. And as for "debugging" I just noticed I committed the version with echo. My bad, will fix.

done

# Install git pre-commit hook
hook:
cp dev/pre-commit `git rev-parse --git-path hooks`
9 changes: 9 additions & 0 deletions dev/pre-commit
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
#!/bin/bash

make format-xml
git add en de
Copy link

Choose a reason for hiding this comment

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

Isn't that problematic though? IIUC this would level-out any differences incurred from the auto-formatter by updating the index already. So now I cannot discern my (presumably manual) changes from the hooked changes anymore.

If you would just omit the git add, then I could still distinguish the index from the new state of the working directory (git diff them).

In fact, that difference is exactly what the line below should check on – instead of distrusting any changes to the *.dita files (which would prevent one from ever changing them manually again, short of circumventing the hook with --no-verify).

Copy link
Member Author

Choose a reason for hiding this comment

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

Isn't that problematic though?

I do not see this as problematic, not being able to differentiate the staged and pre-commit changes. The convenience of just doing "git commit" again outweighs the loss of exact control IMHO.

However you're right, that this would prevent comitting ANY dita files which is of course not what I intended :) I'll adapt to the git diff, don't stage anything approach.

git status --porcelain|grep -qP '^M.*dita$'
if (( $? == 0 ));then
echo "XML needed reformatting, please run 'git commit' again"
exit 1
fi