Robert Parker <[email protected]> writes:
> A couple of days ago a friend of mine who is seriously into web page and
> html5 application development was complaining that none of the CSS
> validators he has been using report on duplicated style names. Some such
> duplication will ruin the rendering of the html in the browser. So I wrote
> a small program in C to find these duplicated style names. It is available
> here:
> https://github.com/rlp1938/Cssdups
Initial reactions:
- you aren't escaping - (as "\-" or "\(em") consistently in the manpage.
- why are you using C, but not using a formal parser? I would think if
you were being quick-and-dirty you'd use perl or awk. If you're
being more robust, use an existing CSS parsing library, or at least
use flex and bison. At *worst* it should be a hand-rolled recdescent
parser, not strchr.
- Also, the GNU Coding Standards explicitly discourages fixed-length
buffers as your "buf[128]". If you are going to use fixed-length
buffers, recommend you make it a macro #def MAX_BUF_LEN 128 at the
top of the file, rather than a magic number in the code body.
- I'm actually a bit puzzled as to why duplicates are faults. Where
does it say that in w3.org/tr/css ? Or are you pandering to some
widespread but non-compliant implementation?
http://ddg.gg/lite?q=site:w3.org+inurl:css+duplicate
- I can't see where you're separating on commas, so for example
h1, h2, h3, h4, h5, h6 { font-family: Helvetica-Narrow }
...would be treated as a single instance of "h1, .... h6", where I
think you should be treating it as separate instances of h1 through
h6. Similarly the matcher can have stuff like "p > pre" and "p pre".
- When testing for being accidentally passed an .html file, I would
also issue a warning if argv[optind] doesn't end in ".css".
- Also recommend either work variadically, or complain loudly when
passed *too many* arguments, i.e. check that argc is 1 after argument
parsin, and complain explicitly.
- The copyright declaration in cssdups.c is not indented consistently.
Recommend you avoid all literal tabs.
> All that's needed after download is enter the directory Cssdups and 'make
> && sudo make install && make clean'
> When you find something better just enter the same dir and 'make remove' to
> remove the program, man page and config files.
That would be "sudo make remove", obviously.
Regarding your makefile, "all" is also a .PHONY. Since I generally
target GNU make only, I would use its built-in CC, .c.o &c rules, since
they will handle correctly variables like CPPFLAGS (which you don't).
Thus (untested):
PREFIX ?= /usr/local
all: cssdups
install: all
install -d $(DESTDIR)/$(PREFIX)/bin/
install cssdups $(DESTDIR)/$(PREFIX)/bin/cssdups
If you want to get fancier as install.sh indicates, I recommend learning
autotools instead of trying to reinvent it. You should definitely
support DESTDIR and PREFIX at a minimum, as otherwise you will piss off
$distro packaging people.
_______________________________________________
luv-main mailing list
[email protected]
http://lists.luv.asn.au/listinfo/luv-main
