[gnuastro-commits] master 6acb864 1/2: CFITSIO type code corrected, maximum option value length

[Mohammad Akhlaghi]

branch: master
commit 6acb864c54cd8013549752ba21be21a563e05c8a
Author: Mohammad Akhlaghi <address@hidden>
Commit: Mohammad Akhlaghi <address@hidden>

    CFITSIO type code corrected, maximum option value length
    
    In `gal_fits_key_write_filename', there was a mistaken reference to
    CFITSIO's `TSTRING' type code rather than Gnuastro's
    `GAL_TYPE_STRING'. This has been corrected.
    
    In Crop, we now increment the input image counter `crp->numimg' in the
    high-level `imgmodecrop' and `wcsmodecrop' functions, not in `onecrop'. The
    reason for this is that the value is ultimately decided and used in these
    high-level functions, so having it in `onecrop' didn't help in readability.
    
    Finally, when printing option values and their short documentation, there
    is now a maximum limit on the length of the value. If a value's length is
    longer, that option's documentation will be one space character away from
    the value (and not aligned). But atleast the other options will be
    reasonable (as described above `GAL_OPTIONS_MAX_VALUE_LEN').
---
 bin/crop/crop.c                 | 11 +++++++++--
 bin/crop/onecrop.c              |  6 ++++--
 lib/fits.c                      | 11 +++++++----
 lib/gnuastro-internal/options.h | 11 ++++++++++-
 lib/options.c                   |  6 ++++--
 5 files changed, 34 insertions(+), 11 deletions(-)

diff --git a/bin/crop/crop.c b/bin/crop/crop.c
index fbebda5..0d12f95 100644
--- a/bin/crop/crop.c
+++ b/bin/crop/crop.c
@@ -209,7 +209,7 @@ imgmodecrop(void *inparam)
       /* Set all the output parameters: */
       crp->out_ind=crp->indexs[i];
       crp->outfits=NULL;
-      crp->numimg=0;
+      crp->numimg=1;   /* In Image mode there is only one input image. */
       cropname(crp);
 
       /* Crop the image. */
@@ -298,7 +298,10 @@ wcsmodecrop(void *inparam)
             /* If a name isn't set yet, set it. */
             if(crp->name==NULL) cropname(crp);
 
-            /* Do the crop. */
+            /* Increment the number of images used (necessary for the
+               header keywords that are written in `onecrop'). Then do the
+               crop. */
+            ++crp->numimg;
             onecrop(crp);
 
             /* Close the file. */
@@ -308,16 +311,20 @@ wcsmodecrop(void *inparam)
           }
       while ( ++(crp->in_ind) < p->numin );
 
+
       /* Correct in_ind. The loop above went until `in_ind' is one more
          than the index for the last input image (that is how it exited the
          loop). But `crp->in_ind' is needed later, so correct it here. */
       --crp->in_ind;
 
+
       /* Check the final output: */
       if(crp->numimg)
         {
+          /* See if the center is filled. */
           crp->centerfilled=iscenterfilled(crp);
 
+          /* Write all the dependency versions and close the file. */
           gal_fits_key_write_version(crp->outfits, NULL, PROGRAM_STRING);
           status=0;
           if( fits_close_file(crp->outfits, &status) )
diff --git a/bin/crop/onecrop.c b/bin/crop/onecrop.c
index cb26620..d9f824e 100644
--- a/bin/crop/onecrop.c
+++ b/bin/crop/onecrop.c
@@ -729,6 +729,7 @@ onecrop(struct onecropparams *crp)
                           &anynul, &status))
         gal_fits_io_error(status, NULL);
 
+
       /* If we have a floating point or double image, pixels with zero
          value should actually be a NaN. Unless the user specificly
          asks for it, make the conversion.*/
@@ -761,7 +762,7 @@ onecrop(struct onecropparams *crp)
       /* A section has been added to the cropped image from this input
          image, so increment crp->imgcount and save the information of
          this image. */
-      sprintf(basename, "ICF%zu", ++crp->numimg);
+      sprintf(basename, "ICF%zu", crp->numimg);
       gal_fits_key_write_filename(basename, img->name, &headers);
       sprintf(regionkey, "%sPIX", basename);
       sprintf(region, "%ld:%ld,%ld:%ld", fpixel_i[0], lpixel_i[0],
@@ -771,6 +772,7 @@ onecrop(struct onecropparams *crp)
                                 "this output.", 0, NULL);
       gal_fits_key_write(ofp, &headers);
 
+
       /* Free the allocated array. */
       free(array);
     }
@@ -778,7 +780,7 @@ onecrop(struct onecropparams *crp)
     if(p->polygon && p->outpolygon==0 && p->mode==IMGCROP_MODE_WCS)
       free(crp->ipolygon);
 
-
+  /* The crop is complete. */
   return;
 }
 
diff --git a/lib/fits.c b/lib/fits.c
index 765f0b8..0f42120 100644
--- a/lib/fits.c
+++ b/lib/fits.c
@@ -1019,7 +1019,7 @@ gal_fits_key_write_filename(char *keynamebase, char 
*filename,
                   filename, maxlength);
 
           /* Convert the last useful character and save the file name.*/
-          gal_fits_key_list_add_end(list, TSTRING, keyname, 1,
+          gal_fits_key_list_add_end(list, GAL_TYPE_STRING, keyname, 1,
                                     value, 1, NULL, 0, NULL);
           i+=j+1;
         }
@@ -1076,7 +1076,7 @@ gal_fits_key_write(fitsfile *fptr, gal_fits_list_key_t 
**keylist)
   tmp=*keylist;
   while(tmp!=NULL)
     {
-      /* Write the information: */
+      /* Write the basic key value and comments. */
       if(tmp->value)
         {
           if( fits_update_key(fptr, gal_fits_type_to_datatype(tmp->type),
@@ -1089,8 +1089,10 @@ gal_fits_key_write(fitsfile *fptr, gal_fits_list_key_t 
**keylist)
           if(fits_update_key_null(fptr, tmp->keyname, tmp->comment, &status))
             gal_fits_io_error(status, NULL);
         }
-      if(tmp->unit && fits_write_key_unit(fptr, tmp->keyname,
-                                          tmp->unit, &status) )
+
+      /* Write the units if it was given. */
+      if( tmp->unit
+          && fits_write_key_unit(fptr, tmp->keyname, tmp->unit, &status) )
         gal_fits_io_error(status, NULL);
 
       /* Free the value pointer if desired: */
@@ -1105,6 +1107,7 @@ gal_fits_key_write(fitsfile *fptr, gal_fits_list_key_t 
**keylist)
       tmp=ttmp;
     }
 
+  /* Set it to NULL so it isn't mistakenly used later. */
   *keylist=NULL;
 }
 
diff --git a/lib/gnuastro-internal/options.h b/lib/gnuastro-internal/options.h
index fde3aa1..caddc38 100644
--- a/lib/gnuastro-internal/options.h
+++ b/lib/gnuastro-internal/options.h
@@ -37,7 +37,16 @@ along with Gnuastro. If not, see 
<http://www.gnu.org/licenses/>.
 /* Type for options that don't accept an argument/value. This macro is
    defined to help make the definition and processing of these options
    easier and less buggy. Please use this macro for such options. */
-#define GAL_OPTIONS_NO_ARG_TYPE GAL_TYPE_UINT8
+#define GAL_OPTIONS_NO_ARG_TYPE   GAL_TYPE_UINT8
+
+/* When printing the option names, values and comments, we want things to
+   be clean and readable (all the comments starting on one line for most,
+   ideally all, lines). But in some cases, option values can become too
+   long (for example the `--polygon' option in Crop, which takes many
+   coordinates). So simply using the maximum option length is going to make
+   the whole thing unreadable and we need to have a maximum so this rule
+   only applies to them. */
+#define GAL_OPTIONS_MAX_VALUE_LEN 10
 
 
 
diff --git a/lib/options.c b/lib/options.c
index 62d300e..1ab94ca 100644
--- a/lib/options.c
+++ b/lib/options.c
@@ -1600,8 +1600,10 @@ options_set_lengths(struct argp_option *poptions,
       options_correct_max_lengths(&coptions[i], &max_nlen, &max_vlen, cp);
 
   /* Save the final values in the output pointers. */
-  *namelen=max_nlen;
-  *valuelen=max_vlen;
+  *namelen  = max_nlen;
+  *valuelen = ( max_vlen < GAL_OPTIONS_MAX_VALUE_LEN
+                ? max_vlen
+                : GAL_OPTIONS_MAX_VALUE_LEN );
 }