[GRASS-dev] SUBMITTING_SCRIPTS and SUBIMITTING_TCLTK patches
Maciek Sieczka
werchowyna at epf.pl
Tue Jun 27 15:55:47 EDT 2006
Hi!
Following your comments I have improved my improvements further.
Let me know what is still wrong.
I have removed the links to BASH tutorials as Markus pointed out
offlist, that we want general SH compatibility, see:
Markus wrote:
> http://grass.itc.it/pipermail/grass-dev/2006-June/023920.html
> http://grass.itc.it/pipermail/grass-dev/2006-June/023939.html
(I have also modified SUBMITTING_SCRIPTS according to the former link)
What shell tutorial would you suggest?
Maciek
------------------------------------------------------------------------
CIEP?E KRAJE - CIEP?E MORZA. Szukasz atrakcyjnego wypoczynku w przyst?pnej cenie, zapoznaj si? z nasz? ofert?.
ZAPRASZAMY
www.skarpatravel.pl
-------------- next part --------------
Index: SUBMITTING_SCRIPTS
===================================================================
RCS file: /home/grass/grassrepository/grass6/SUBMITTING_SCRIPTS,v
retrieving revision 1.6
diff -u -r1.6 SUBMITTING_SCRIPTS
--- SUBMITTING_SCRIPTS 18 May 2006 04:37:03 -0000 1.6
+++ SUBMITTING_SCRIPTS 27 Jun 2006 19:53:16 -0000
@@ -1,10 +1,8 @@
-$Id: SUBMITTING_SCRIPTS,v 1.6 2006/05/18 04:37:03 hamish Exp $
-
NOTE: Please improve this list!
Dear (new) GRASS Developer,
-When submitting SHELL SCRIPTS to GRASS CVS repositiory,
+When submitting SHELL SCRIPTS to GRASS CVS repository,
please take care of following rules:
[ see SUBMITTING for C code hints ]
@@ -14,18 +12,20 @@
module's help page.
http://grass.ibiblio.org/grass61/manuals/html61_user/g.parser.html
+
1. Use the directory structure to place your script appropriately into
the source tree
- scripts go into scripts/
- Consider to take a look at [please suggest Shell tutorial]
+ Consider taking a look at [please suggest Shell tutorial]
+
2. Add a header section to the script you submit and make sure you include
the copyright. The purpose section is meant to contain a general
overview of the code in the file to assist other programmers that will
need to make changes to your code.
- Example (ficticious header for a script called r.myscript) :
+ Example (fictitious header for a script called r.myscript):
#!/bin/sh
@@ -45,31 +45,32 @@
The copyright protects your rights according to GNU General Public
License (www.gnu.org).
+
3. - deleted.
We don't want the $ ID $ in scripts any more as it
causes problems for the CVS branches.
+
4. As a general principle, shell variables should almost always be quoted.
- Use only secure temp files, see g.tempfile and scripts/* for examples.
-5. If you search for a command in $PATH, do NOT
- use the "which" command or the "type -p" command. Both commands are not
- supported on all platforms, thus causing problems for some people. As an
- alternative, please use code similar to the following shell script snippet
- which will perform the same function. In this case, the path of the grass60
- command is saved if grass60 is found in $PATH. This won't recognize aliased
- command name.
- # Search for grass5 command in user's path
+5. If you search for a command in $PATH, do NOT use the "which" command or the
+ "type -p" command. Both commands are not supported on all platforms, thus
+ causing problems for some people. As an alternative, please use code similar
+ to the following shell script snippet which will perform the same function.
+ In this case, the path of the grass61 command is saved if grass61 is found
+ in $PATH. This won't recognize aliased command name.
+
+ # Search for grass61 command in user's path
for i in `echo $PATH | sed 's/^:/.:/
s/::/:.:/g
s/:$/:./
s/:/ /g'`
do
- if [ -f $i/grass5 ] ; then
+ if [ -f $i/grass61 ] ; then
- # Save the path of the grass60 command
- GRASS_PATH=$i/grass60
+ # Save the path of the grass61 command
+ GRASS_PATH=$i/grass61
# Use the first one in user's path
break
fi
@@ -81,32 +82,44 @@
# check if we have awk
if [ ! -x "`which awk`" ] ; then
echo "ERROR: awk required, please install awk or gawk first" 1>&2
- exit 1
+ exit 1g
fi
</?>
+
6. Add a test to check if the user is in GRASS before starting main part
- of script. Result of running the script is unpredicable otherwise.
+ of script. Result of running the script is unpredictable otherwise.
if [ -z "$GISBASE" ] ; then
echo "You must be in GRASS GIS to run this program." 1>&2
exit 1
fi
+
7. Create and use secure temporary files and directories. Use the g.tempfile
module to do this. e.g.
- # setup temporary file
+ # set up temporary file
TMP="`g.tempfile pid=$$`"
if [ $? -ne 0 ] || [ -z "$TMP" ] ; then
echo "ERROR: unable to create temporary files" 1>&2
exit 1
fi
- For temportary directories remove the newly created file and mkdir using
+ For temporary directories remove the newly created file and mkdir using
the same name. Beware of commands like "rm -f ${TMP}*" as this becomes
"rm -f *" if $TMP is unset (hence the test above).
+ Also, a good idea is to include your script's name in the temporary files
+ names. It makes it easy to spot orphans. E.g.:
+
+ # set a variable according to your script's name
+ PROG=`basename $0`
+
+ # now create temp files e.g. like this:
+ v.out.ascii input="${OUTPUT}" format=standard > "${TMP}_${PROG}.out"
+
+
8. Testing the existence of variables. For portability, use
if [ -z "$VARIABLE" ] ; then
instead of
@@ -126,9 +139,11 @@
# set environment so that awk works properly in all languages
unset LC_ALL
- export LC_NUMERIC=C
+ LC_NUMERIC=C
+ export LC_NUMERIC
- awk '{print $1}'
+ Note: BASH allows the last 2 lines to be reduced to
+ "export LC_NUMERIC=C", but that isn't portable, so don't use it.
10. Use g.findfile when there is a need to test if a map exists.
@@ -140,12 +155,57 @@
exit 1
fi
-11. Send solely informational output to stderr, not stdout.
- echo "Done." 1>&2
-12. For consistency, use README rather than README.txt for any README files.
+11. Send output containing status information to stderr, not stdout. stdout
+ is reserved for parsable output. E.g.:
+
+ echo "$PROG complete." 1>&2
+
+
+12. Consider redirecting irrelevant stdout of "noisy" modules used in your
+ script to /dev/null. E.g.:
+
+ g.remove rast="${TMP}_${PROG}" > /dev/null
+
+ Grass modules send error and warning messages to stderr, so they will
+ not be hidden by the muting of stdout.
-13. Be sure to develop on top of the LATEST GRASS code (which is in CVS).
+
+13. Scripts shouldn't modify the current region, unless it is the script's
+ intended function. If the same region is required for subsequent Grass
+ commands, use WIND_OVERRIDE or GRASS_REGION Grass variables instead.
+ E.g.:
+
+ # create a unique name for the saved region, following the naming hints
+ # above:
+
+ working_region="${TMP}_${PROG}.region"
+
+ # set the region; mind "-u" switch is used - allows for *not* modifying
+ # the current region, which is the point here:
+
+ g.region -u -a rast=some_raster save="$working_region"
+
+ # make the region persistent during the whole script runtime:
+
+ WIND_OVERRIDE="$working_region"
+ export WIND_OVERRIDE
+
+ ... script body ...
+
+ # clean up:
+
+ unset WIND_OVERRIDE
+ g.remove region="$working_region"
+
+ Refer to "GRASS variables and environment variables" documentation section
+ for more info.
+
+
+14. For consistency, use README rather than README.txt for any README files.
+
+
+15. Be sure to develop on top of the LATEST GRASS code (which is in CVS).
You can re-check before submission with 'cvs diff':
Be sure to create unified ("diff -u") format. "Plain"
@@ -157,13 +217,19 @@
"cvs diff -u display/d.vect/main.c"; that way, the diff will
include the pathname rather than just "main.c".
-14. Tell the other developers about the new code using the following e-mail:
+
+16. Tell the other developers about the new code using the following e-mail:
grass-dev at grass.itc.it
To subscribe to this mailing list, see
http://grass.itc.it/devel/index.php
-15. In case of questions feel free to contact the developers at the above
+
+17. Consider submitting your script to Grass add-ons WIKI section:
+ http://grass.gdf-hannover.de/wiki/GRASS_AddOns#GRASS_6.x
+
+
+18. In case of questions feel free to contact the developers at the above
mailing list.
http://grass.itc.it/devel/index.php#submission
-------------- next part --------------
Index: SUBMITTING_TCLTK
===================================================================
RCS file: /home/grass/grassrepository/grass6/SUBMITTING_TCLTK,v
retrieving revision 1.4
diff -u -r1.4 SUBMITTING_TCLTK
--- SUBMITTING_TCLTK 5 May 2006 18:36:43 -0000 1.4
+++ SUBMITTING_TCLTK 25 Jun 2006 11:31:13 -0000
@@ -120,35 +120,11 @@
by options.tcl. You can include it at the start of your script with:
source $env(GISBASE)/etc/gtcltk/options.tcl
-10. Avoid using global variables. Thay are a frequent source of bugs, make code
+10. Avoid using global variables. They are a frequent source of bugs, make code
harder to understand, and make your program difficult to reuse. Additionally,
putting code into procs usually makes it run faster (it gets compiled).
-11. For consistency, use README rather than README.txt for any README files.
-
-12. Be sure to develop on top of the LATEST GRASS code (which is in CVS).
- You can re-check before submission with 'cvs diff':
-
- Be sure to create unified ("diff -u") format. "Plain"
- diffs (the default format) are risky, because they will apply without
- warning to code which has been substantially changed; they are also
- harder to read than unified.
-
- Such diffs should be made from the top-level directory, e.g.
- "cvs diff -u display/d.vect/main.c"; that way, the diff will
- include the pathname rather than just "main.c".
-
-13. Tell the other developers about the new code using the following e-mail:
- grass-dev at grass.itc.it
-
- To subscribe to this mailing list, see
- http://grass.itc.it/devel/index.php
-
-14. In case of questions feel free to contact the developers at the above
- mailing list.
- http://grass.itc.it/devel/index.php#submission
-
-15. Try to evaluate things only once. Tcl compiles the program to bytecode which
+11. Try to evaluate things only once. Tcl compiles the program to bytecode which
can be interpreted fairly quickly. If there are strings that must still be
evaluated tcl must parse and either compile or interpret them
each time they are encountered. In general this means put braces around
@@ -178,7 +154,7 @@
multiple times store it in a variable that will not be destroyed or
changed between reuse. Tcl stores the compiled regex with the variable.
-16. You might want to decompose lists in a somewhat easy way:
+12. You might want to decompose lists in a somewhat easy way:
Difficult and slow:
# Make x1 y1 x2 y2 the first four things in the list
@@ -194,7 +170,7 @@
Be sure to include a comment as to what you are doing.
-17. Use the Tcl list functions (list, lappend etc) for manipulating lists.
+13. Use the Tcl list functions (list, lappend etc.) for manipulating lists.
For example, use:
@@ -210,18 +186,42 @@
because tcl is not internally converting between strings and lists.
A related issue is to remember that command lines (as used by exec and
- open "|...") are lists. exec behaves like execl(), spawnl() etc, and
+ open "|...") are lists. exec behaves like execl(), spawnl() etc., and
not like system().
Overlooking either of these points is likely to result in code which
fails when a command argument contains a space.
-18. Tcl C library:
+14. Tcl C library:
Memory allocated with Tcl_Alloc (such as the result of Tcl_Merge)
must be freed with Tcl_Free. This means that the ->freeProc of
an interpreter when returning a string using Tcl_Merge should be
TCL_DYNAMIC. Incorrectly freeing memory with glibc free will
cause segfaults in Tcl 8.4 and later.
+
+15. For consistency, use README rather than README.txt for any README files.
+
+16. Be sure to develop on top of the LATEST GRASS code (which is in CVS).
+ You can re-check before submission with 'cvs diff':
+
+ Be sure to create unified ("diff -u") format. "Plain"
+ diffs (the default format) are risky, because they will apply without
+ warning to code which has been substantially changed; they are also
+ harder to read than unified.
+
+ Such diffs should be made from the top-level directory, e.g.
+ "cvs diff -u display/d.vect/main.c"; that way, the diff will
+ include the pathname rather than just "main.c".
+
+17. Tell the other developers about the new code using the following e-mail:
+ grass-dev at grass.itc.it
+
+ To subscribe to this mailing list, see
+ http://grass.itc.it/devel/index.php
+
+18. In case of questions feel free to contact the developers at the above
+ mailing list.
+ http://grass.itc.it/devel/index.php#submission
...
[please add further hints if required]
More information about the grass-dev
mailing list