Re: Issue 5739: Add makefile targets for formatting all C++ code (issue 565620043 by nine.fierce.ball...@gmail.com)

[nine . fierce . ballads]

On 2020/02/05 17:52:20, dak wrote:
> configure.ac:367: STEPMAKE_PROGS(CLANG_FORMAT, clang-format-9
clang-format,
> OPTIONAL, 9, 9)
> Interesting.  This gives a warning when it isn't installed?  I think
that we
> don't usually flag dependencies of components not involved in either
building or
> running LilyPond.  For example, we provide Emacs and vi style files
without
> checking for availability of either editor.

Yes, it gives a warning.  If this is a problem, IMO its is a shortcoming
of the configuration system: failing to distinguish between simply
optional programs and strongly recommended programs.  There's a similar
issue with tidy.  If it's there, we want to take advantage of it, but
warning the dev that it is missing is a nuisance.


https://codereview.appspot.com/565620043/

Re: Issue 5739: Add makefile targets for formatting all C++ code (issue 565620043 by nine.fierce.ball...@gmail.com)

[dak]

https://codereview.appspot.com/565620043/diff/549530043/GNUmakefile.in
File GNUmakefile.in (right):

https://codereview.appspot.com/565620043/diff/549530043/GNUmakefile.in#newcode319
GNUmakefile.in:319: ## TODO: This condition speeds up make when
formatting targets are not
I would not really work with a timestamp file but rather use git status
to detect modified files, so that only those would be affected.  That,
however, is more a useful approach for a script rather than a Makefile.

https://codereview.appspot.com/565620043/diff/549530043/configure.ac
File configure.ac (right):

https://codereview.appspot.com/565620043/diff/549530043/configure.ac#newcode367
configure.ac:367: STEPMAKE_PROGS(CLANG_FORMAT, clang-format-9
clang-format, OPTIONAL, 9, 9)
Interesting.  This gives a warning when it isn't installed?  I think
that we don't usually flag dependencies of components not involved in
either building or running LilyPond.  For example, we provide Emacs and
vi style files without checking for availability of either editor.

https://codereview.appspot.com/565620043/

Issue 5739: Add makefile targets for formatting all C++ code (issue 565620043 by nine.fierce.ball...@gmail.com)

[nine . fierce . ballads]

Reviewers: ,

Description:
https://sourceforge.net/p/testlilyissues/issues/5739/

These are not yet intended for routine use by contributors.  They are
meant to help explore the differences between astyle and clang-format.

Please review this at https://codereview.appspot.com/565620043/

Affected files (+48, -0 lines):
  M GNUmakefile.in
  M config.make.in
  M configure.ac


Index: GNUmakefile.in
diff --git a/GNUmakefile.in b/GNUmakefile.in
index 
d63850fde6a4fb163f0b77c6c0abbae3accd3ac3..0d570c65456ea6342cd363795ba0bee7878cf972
 100644
--- a/GNUmakefile.in
+++ b/GNUmakefile.in
@@ -313,6 +313,50 @@ grand-replace:
PATH=$(buildscript-dir):$(PATH) $(buildscript-dir)/grand-replace
 
 
+
+# code formatting
+
+## TODO: This condition speeds up make when formatting targets are not
+## going to be run.  Explore alternatives (lazy init?).
+ifneq (,$(filter format%,$(MAKECMDGOALS)))
+
+CXX_STYLE_FILES := \
+   $(wildcard $(top-src-dir)/*/*/*[ch]) \
+   $(wildcard $(top-src-dir)/*/*.cc)
+# end list
+
+# Format updated source files using astyle.
+ASTYLE_STAMP:=$(top-build-dir)/.astyle-done
+.PHONY: format-all-astyle
+format-all-astyle: $(ASTYLE_STAMP)
+$(ASTYLE_STAMP): $(CXX_STYLE_FILES)
+#   format the source files that are newer than the timestamp file
+   $(call ly_progress,Making,$@,($(words $?) files))
+#   TODO: fixcc complains if more than 4 files are given
+   echo $? | xargs -n4 $(top-src-dir)/scripts/auxiliar/fixcc.py
+   touch $@
+
+# Format updated source files using clang-format.
+CLANG_FORMAT_STAMP:=$(top-build-dir)/.clang-format-done
+.PHONY: format-all-clang
+format-all-clang: $(CLANG_FORMAT_STAMP)
+$(CLANG_FORMAT_STAMP): $(CXX_STYLE_FILES)
+#   format the source files that are newer than the timestamp file
+   $(call ly_progress,Making,$@,($(words $?) files))
+   $(CLANG_FORMAT) -i $?
+   touch $@
+
+# Format updated source files using one of multiple approved tools,
+# depending on what is installed.
+.PHONY: format-all
+ifneq ($(CLANG_FORMAT),false)
+format-all: format-all-clang
+else
+format-all: format-all-astyle
+endif
+
+endif
+
 
 # testing
 
Index: config.make.in
diff --git a/config.make.in b/config.make.in
index 
8825c909e3fc0de699e5b952b69e1c183d436250..04debde0720a845c2e8847ca1f8dd5ded209fd8e
 100644
--- a/config.make.in
+++ b/config.make.in
@@ -107,6 +107,7 @@ AR = @AR@
 BASH = @BASH@
 BISON = @BISON@
 CC = @CC@
+CLANG_FORMAT = @CLANG_FORMAT@
 CONFIGSUFFIX = @CONFIGSUFFIX@
 CROSS = @cross_compiling@
 CXX = @CXX@
Index: configure.ac
diff --git a/configure.ac b/configure.ac
index 
72bd994740e85f2278ffc6c1368ce6ac0bdcaddd..a87c80339fdfb713d5d812649716c5f9398e5b36
 100644
--- a/configure.ac
+++ b/configure.ac
@@ -363,6 +363,9 @@ fi
 # perl for help2man and for mf2pt1.pl
 STEPMAKE_PERL(REQUIRED)
 
+# code formatting
+STEPMAKE_PROGS(CLANG_FORMAT, clang-format-9 clang-format, OPTIONAL, 9, 9)
+
 # tidy can be used to validate the HTML pages produced during
 # regression testing
 STEPMAKE_PATH_PROG(TIDY, tidy, OPTIONAL)