GNOME Bugzilla – Bug 663576
make -C plug-ins/script-fu check-for-deprecated-procedures-in-script-fu lists files that do not use deprecated functions
Last modified: 2018-01-01 13:26:04 UTC
See bug #647834, comment #9. grep treats - as a non-word character, so `grep -w gimp-text` finds gimp-text-font, for example. Moreover, some calls are commented out, so we should not worry about them. This patch fixes that. Also, I replaced the inner loop with a regexp, so now it is much faster.
Created attachment 200902 [details] [review] patch
The original version could have been fixed by appending \\s to the regexp used to look for the procedure name. The patch adds the requirement that the machine used to build GIMP must have tr, sed, cut, and uniq in addition to grep to use the check-for-deprecated-procedures-in-script-fu make target. Is it safe to assume that all environments used to build GIMP will have these utilities programs?
Well, it uses for, if and grep already, which I think are UNIX specific, so I thought it was meant to be used in environments that have these. If not, it should use perl or python or whatever is available. However, \s cannot be used with grep unless you specify -P (perl-regexp); the POSIX equivalent is [[:blank:]]. And we should prepend a ( to the procedure's name so we do not find whatever-gimp-text (although it may not be necessary since all names begin with gimp-). The cut part is useful too, since the diagnosis should not warn about names appearing in comments. A Perl regexp can do it, but I do not think a POSIX regexp can. Getting rid of the inner loop only make it faster, so it is not as useful.
Created attachment 201046 [details] [review] smaller patch Uh seems like my brain was asleep when I wrote this: the cut part can easily be replaced with ^[^;]* at the beginning of the regexp. This new patch changes only the pattern that grep looks for. Also this time I made sure that it works when the procedure takes no parameter or appear at the end of a line. Anyway, what is most important for now is that the bug #647834 got fixed, and this issue can wait until next time the API changes. I use almost exclusively Linux, so I cannot tell if it is OK to use other tools like tr, sed, cut and uniq. I am leaving my previous patch here because of how much faster it is.
Comment on attachment 200902 [details] [review] patch per comment 4
Review of attachment 201046 [details] [review]: The idea behind the patch is good but the actual patch doesn't look right. The code being patched is being used to check Script-Fu scripts. It needs to look for the open parenthesis, followed by optional whitespace, the procedure name, then non-optional whitespace.
Kevin, can you just push something that makes the matching work and close this bug please?
Created attachment 366111 [details] [review] patch This new regex is a little stronger than the previous one (it does not match e.g. whatever-gimp-levels). It looks for: - no semicolon (to ignore comments) - whitespace OR opening parenthesis OR beginning of line - the procedure name - whitespace OR closing parenthesis OR end of line It does not require an opening parenthesis because: - the opening parenthesis might be on the previous line - the procedure might be used as an argument to map or reduce
Thanks! This is minor enough to be on the 2.10 milestone :)
Fixed in master: commit a51efd09d12b4a40de8e3d0ae169f004be1cf1d1 Author: Alexis Wilhelm <alexiswilhelm@gmail.com> Date: Sat Dec 30 13:09:14 2017 +0100 Bug 663576 - make -C plug-ins/script-fu check-for-deprecated-procedures-in-script-fu... ...lists files that do not use deprecated functions Better regex that matches the right stuff. plug-ins/script-fu/Makefile.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)