# HG changeset patch # User markus schnalke # Date 1340722895 -7200 # Node ID e49780100ffb8ea203cd56f2580163c1f553a1ea # Parent f9bf4d5ac1ba7f390ca93a455d0df934dcfd0acb Wrote about the rework of anno(1). diff -r f9bf4d5ac1ba -r e49780100ffb discussion.roff --- a/discussion.roff Tue Jun 26 17:01:10 2012 +0200 +++ b/discussion.roff Tue Jun 26 17:01:35 2012 +0200 @@ -2540,17 +2540,23 @@ demands: ``Don't belabor the obvious.'' Hence, I simply removed comments like the following: .VS -context_replace(curfolder, folder); /* update current folder */ -context_save(); /* save the context file */ -folder_free(mp); /* free folder structure */ +context_replace(curfolder, folder); /* update current folder */ +seq_setcur(mp, mp->lowsel); /* update current message */ +seq_save(mp); /* synchronize message sequences */ +folder_free(mp); /* free folder/message structure */ +context_save(); /* save the context file */ + +[...] + +int c; /* current character */ +char *cp; /* miscellaneous character pointer */ + +[...] + +/* NUL-terminate the field */ +*cp = '\0'; VE -.Ci 8edc5aaf86f9f77124664f6801bc6c6cdf258173 -.VS -seq_setcur (mp, mp->lowsel);/* set current message for folder */ -seq_save (mp); /* synchronize message sequences */ -folder_free (mp); /* free folder/message structure */ -VE -.Ci 337338b404931f06f0db2119c9e145e8ca5a9860 +.Ci 426543622b377fc5d091455cba685e114b6df674 .P The names of the functions explain enough already. @@ -2623,17 +2629,174 @@ .P At the end of their chapter on style, Kernighan and Pike ask: ``But why worry about style?'' -The following description of my rework on +The following description of my rework of .Pn anno -shows why style matters. +should give answers. .P +Until 2002, +.Pn anno +had six functional command line switches, +.Sw -component +and +.Sw -text , +which took an argument each, +and the two pairs of flags, +.Sw -[no]date +and +.Sw -[no]inplace., +.Sw -component +and +.Sw -text , +which took an argument each, +and the two pairs of flags, +.Sw -[no]date +and +.Sw -[no]inplace . +Then Jon Steinhart introduced his attachment system. +In need for more advanced annotation handling, he extended +.Pn anno . +He added five more switches: +.Sw -draft , +.Sw -list , +.Sw -delete , +.Sw -append , +and +.Sw -number , +the last one taking an argument. +Later, +.Sw -[no]preserve +was added. +Then, the Synopsis section of the man page +.Mp anno (1) +read: +.VS +anno [+folder] [msgs] [-component field] [-inplace | -noinplace] + [-date | -nodate] [-draft] [-append] [-list] [-delete] + [-number [num|all]] [-preserve | -nopreserve] [-version] + [-help] [-text body] +VE +.LP +The implementation followed the same structure. +Problems became visible when +.Cl "anno -list -number 42 +worked on the current message instead on message number 42, +and +.Cl "anno -list -number l:5 +did not work on the last five messages but failed with the misterious +error message: ``anno: missing argument to -list''. +Yet, the invokation matched the specification in the man page. +There, the correct use of +.Sw -number +was defined as being +.Cl "[-number [num|all]] +and the textual description for the combination with +.Sw -list +read: +.QS +The -list option produces a listing of the field bodies for +header fields with names matching the specified component, +one per line. The listing is numbered, starting at 1, if +the -number option is also used. +.QE +.LP +The problem was manifold. +The code required a numeric argument to the +.Sw -number +switch. +If it was missing or non-numeric, +.Pn anno +aborted with an error message that had an off-by-one error, +printing the switch one before the failing one. +Semantically, the argument to the +.Sw -number +switch is only necessary in combination with +.Sw -delete , +but not with +.Sw -list . +In the former case it is even necessary. +.P +Trying to fix these problems on the surface would not have solved it truly. +The problems discovered originate from a discrepance between the semantic +structure of the problem and the structure implemented in the program. +Such structural differences can not be cured on the surface. +They need to be solved by adjusting the structure of the implementation +to the structure of the problem. +.P +In 2002, the new switches +.Sw -list +and +.Sw -delete +were added in the same way, the +.Sw -number +switch for instance had been added. +Yet, they are of structural different type. +Semantically, +.Sw -list +and +.Sw -delete +introduce modes of operation. +Historically, +.Pn anno +had only one operation mode: adding header fields. +With the extension, it got two moder modes: +listing and deleting header fields. +The structure of the code changes did not pay respect to this +fundamental change to +.Pn anno 's +behavior. +Neither the implementation nor the documentation did clearly +define them as being exclusive modes of operation. +Having identified the problem, I solved it by putting structure into +.Pn anno +and its documentation. +.Ci d54c8db8bdf01e8381890f7729bc0ef4a055ea11 +.P +The difference is visible in both, the code and the documentation. +The following code excrept was replaced: +.VS +int delete = -2; /* delete header element if set */ +int list = 0; /* list header elements if set */ +[...] +case DELETESW: /* delete annotations */ + delete = 0; + continue; +case LISTSW: /* produce a listing */ + list = 1; + continue; +VE +At its place moved the following code: +.VS +static enum { MODE_ADD, MODE_DEL, MODE_LIST } mode = MODE_ADD; +[...] + +case DELETESW: /* delete annotations */ + mode = MODE_DEL; + continue; +case LISTSW: /* produce a listing */ + mode = MODE_LIST; + continue; +VE +.LP +The replacement code does not only match the problem's structure, +but it is easier to understand as well. .P -goto -.P -anno rework +The man page was completely reorganized to propagate the same structure. +This is already visible in the Synopsis section: +.VS +anno [+folder] [msgs] [-component field] [-text body] + [-append] [-date | -nodate] [-preserve | -nopreserve] + [-Version] [-help] + +anno -delete [+folder] [msgs] [-component field] [-text + body] [-number num | all ] [-preserve | -nopreserve] + [-Version] [-help] + +anno -list [+folder] [msgs] [-component field] [-number] + [-Version] [-help] +VE