Would you be so kind as to describe your views here. As I said, I have no real experience writing scheme, and would appreciate comments on style (and construction details) so I may learn.
OK, here is some style feedback. Mind, these are just my opinions and though I will try to present some logic to support them, there are certainly valid reasons to do things in different manners. I want to emphasize that it is not my intent to criticize your choices -- indeed your script is better written than much of the Script-fu code extant, including some of my own -- but to just offer some of the things I consider when writing my own scripts.
I don't understand the reasoning behind the "HACK". Your script needs to define at least one global function, and any definitions made within that global function are local to that function. What does wrapping a definition such as
(define suffix2 "#Rotated") within a
let accomplish that wouldn't be accomplished by placing it inside a global
define?
The naming scheme for the variables and procedures seems a somewhat haphazard mixture employing full words, abbreviations, CamelCase, and punctuation. My personal preference is for lowercase-only, full words separated with hyphens when necessary (and trying to be consistent with the naming of Scheme functions and PDB procedures); but regardless of approach, more consistency should improve your code's readability. For what it's worth, I myself am not particularly proficient at naming things and I certainly have made some horrendous choices in my own scripts.
Here is a smattering of some of your choices with my opinion on them:
- image, layer, spacing : excellent choices. As much as reasonable, I try to use argument names as they are provided by the PDB documentation. This has a (minor) added benefit when using the "Apply" button from within the Script-fu Console's "Browse" dialog.
- basebrush, workbrush : my preference would be to separate the "adjective" (or "attribute") from the "object" using hyphens (base-brush, work-brush). This separation is sometimes not perfectly clear. For example, by convention 'filename' (or 'layername', or 'brushname' etc) seem acceptable as "objects" in and of themselves, and so they do not feel that objectionable; whereas cases such as a layer's width call for more distinction ('layer-width' versus 'layerwidth').
- brushWidth, centerX : my preference would be to keep things lowercase, and use hyphens for separation (brush-width, center-x). I don't find the capitalization of X and Y to be that objectionable, but tend to keep things lowercase for consistency. FWIW, I do not object to camelcase in general -- I actually consider it an elegant approach -- but its employment in Scheme code is not at all common and using it in Script-fu scripts leads to inconsistency (as would any usage of underscores).
- binfo, bwidth, bspacing : I would probably expand these out to make it clear that they are brush attributes. If it is clear that the attribute is associated with the object, I might omit the object (e.g., just use 'spacing' instead of 'brush-spacing'). Likewise, since the width and height are associated with not only the brush but with the image and layer dimensions, I see no confusion arising from just using unadorned versions ('width', 'height').
- bbase, bsave : these are not that descriptive of what they are (text strings/names).
- brush-isgray? : good use of the eroteme to indicate a function returning a boolean per Scheme convention. I would probably just go with 'brush-gray?' as it feels more consistent with other, similar tests ('zero?', 'pair?', 'integer?'). Basically the question mark indicates the nature of the function.
- rotate!, flip! : I imagine that your goal in using the exclamation mark was to follow Scheme convention for procedures that may result in modification of the arguments (a la 'set!'). If this is so, I don't really agree that the arguments are being modified. In these functions, the modification is moreso to the graphical contents of the image or layer, and not to their "handles" (identification value) as manipulated by the function. In the case of 'rotate!', the layer's handle does get modified (destroyed) but this is not really the purpose of the function.
------------------------------------
Moving on, I find your indentation style quite good and very consistent with the preferred Scheme conventions. As for myself, I tend to deviate from convention somewhat significantly in a couple of regards. I am not recommending that you adopt my unconventional conventions but present them nonetheless as thought food.
First, I tend to forgo the "indent to align" practice and just tab in one level (two spaces). The reasoning behind this is that the function names typically appearing in Script-fu are much longer than might traditionally appear in Scheme code. It is all well and good to indent past a function such as 'write' or 'cons' but doing so for a function named 'mathmap-distorts-inverse-lambert-azimuthal-projection' or somesuch quickly forces all subsequent lines to be crammed against the right edge of your text editor (possibly requiring horizontal scrolling to see the entirety of the code block).
I admit that my approach creates almost as many problems as it resolves but since I have a decent text editor that highlights the vertical alignment of matching parenthesis, I prefer it to using long lines of text and having the majority of my text screen blank. I also deviate from my own indentation style (and thus more closely follow Scheme convention) in the case of functions which have many arguments as shown in the following code block:
(gimp-drawable-transform-rotate-default layer
(* new-cumulation (/ *pi* 180))
TRUE
0 0
TRUE
TRANSFORM-RESIZE-ADJUST) While I'm thinking of it, I would also highly recommend that you use the Script-fu enumerated constants (TRUE, FALSE, RGB-MODE, etc) whenever you are able as it is quite helpful in making the code more readable and in preventing silly mistakes when later revising your scripts. It also serves as good educational rote which aids in learning the functionality and interface of the various PDB functions.
The second area where I often deviate from conventional Scheme style guidelines is in accumulating nested right parentheses together on the last line of a code block. I generally keep these separate:
;; My approach
;
(while (not done?)
(if changed?
(begin
(save file)
(delete image)
(set! count (+ count 1))
)
(begin
(delete image)
)
)
)
;; Conventional Scheme
;
(while (not done?)
(if changed?
(begin
(save file)
(delete image)
(set! count (+ count 1)))
(begin
(delete image))))I do this not because I consider my approach more readable, but because it permits me to easily copy-n-paste my script's code from the text editor to GIMP's Script-fu Console to test code fragments (line-by-line) during development. The conventional Scheme style would require that I select portions of a line (positioning the cursor at the correct location within the string of right parentheses) whereas my approach generally permits selecting entire lines or groups of lines. I actually greatly prefer the readability of the conventional approach -- and will sometimes convert my fully-vetted scripts to use it -- but given the rather limited debugging tools available with Script-fu, the advantages while developing my script justify my unconventional method.
I have only one more major comment which I will make in a subsequent post, but for now I will wrap up by addressing two trivial issues.
First, in your comments you raise the question as to why conversion to grayscale and flattening must come *after* the layer rotation. The answer is that rotating a layer non-orthogonally results in an alpha channel being added to the layer.
Second, when you are flattening the image, you first save the foreground and background so as to later restore them. Ignoring the fact that there is no need to save the foreground color, this preservation of the original background can more readily be accomplished with a 'gimp-context-push' / 'gimp-context-pop' wrapping. It is possibly owing to the example provided in some of my scripts wherein you acquired the less-than-optimal approach. I originally adopted my bad habit back during GIMP 2.2 because of a bug in 'gimp-context-push/pop' failing to record the FG and BG colors. There is no longer any reason to avoid using 'gimp-context-push/pop' (and using it saves a few lines a code and additional variables).
In a future installment I shall cover factoring of procedures.
