Thread: Use a pager to show the help files

Started: 2008-09-14 10:41:41
Last activity: 2008-09-17 05:52:31
Topics: SAC Developers
Kuang He
2008-09-14 10:41:41
Hi all,

Attached is a patch (against v101.2) to make SAC use a pager specified
by environment variable $PAGER, instead of the current home-grown tool
similar to ``more'', to show the help files. The good thing is that in
this way, people get to use their favorite pager when viewing the help
files. I personally use ``less'', and its biggest advantage over
``more'' is that it can support scrolling backwards using hotkeys like
Up Arrow, PageDown, Ctrl+U, k, etc. Plus, I get to scroll either way
by a few lines or by half a page, i.e. not necessarily by a whole
page.

Known caveat:

- When the user have an unreasonable setting of $PAGER, the error
message of not being able to load the pager might be a bit mysterious
to the users.

$ export PAGER=xxx
$ sac
SAC> h fg
sh: xxx: not found

- I did not check the return value of malloc() in my code, since I was
hoping someone could come up with a uniform way of reporting malloc()
errors so that we could all stick to it. One possible way is to have a
warpper function to malloc(), and use that instead of malloc() all the
time. And yes, I am aware that there are SO MANY malloc()'s in the
code, whose return values have not been checked!

void *
xmalloc(size_t size)
{
void *p;

if ((p = malloc(size)) == NULL)
fprintf(stderr, "Malloc failed: %s", strerror(errno));
return (p);
}


Best regards,

--
Kuang He
Department of Physics
University of Connecticut
Storrs, CT 06269-3046

Tel: +1.860.486.4919
Web: http://www.phys.uconn.edu/~he/

Attachments
  • Brian Savage
    2008-09-14 19:04:13
    Kuang He

    Looking over the patches I have a couple of suggestions/comments.

    Could we use ANSI C syntax rather than K&R for function declarations.
    This would allow for the easy creation of header files by copying and
    it is a current/standard notation.

    Pager use: We need to check the system for the less command on
    compilation. In fact we could check for less, then more, then
    fallback to the original code in sac. A change like this might allow
    for syntax highlighting in the help files. This would mean the
    current paging code needs to be made into a seperate function. The
    pager value could always be checked for to override the default.

    Cheers
    Brian

    On Sep 14, 2008, at 3:41 AM, "Kuang He" <icrazy<at>gmail.com> wrote:

    Hi all,

    Attached is a patch (against v101.2) to make SAC use a pager specified
    by environment variable $PAGER, instead of the current home-grown tool
    similar to ``more'', to show the help files. The good thing is that in
    this way, people get to use their favorite pager when viewing the help
    files. I personally use ``less'', and its biggest advantage over
    ``more'' is that it can support scrolling backwards using hotkeys like
    Up Arrow, PageDown, Ctrl+U, k, etc. Plus, I get to scroll either way
    by a few lines or by half a page, i.e. not necessarily by a whole
    page.

    Known caveat:

    - When the user have an unreasonable setting of $PAGER, the error
    message of not being able to load the pager might be a bit mysterious
    to the users.

    $ export PAGER=xxx
    $ sac
    SAC> h fg
    sh: xxx: not found

    - I did not check the return value of malloc() in my code, since I was
    hoping someone could come up with a uniform way of reporting malloc()
    errors so that we could all stick to it. One possible way is to have a
    warpper function to malloc(), and use that instead of malloc() all the
    time. And yes, I am aware that there are SO MANY malloc()'s in the
    code, whose return values have not been checked!

    void *
    xmalloc(size_t size)
    {
    void *p;

    if ((p = malloc(size)) == NULL)
    fprintf(stderr, "Malloc failed: %s", strerror(errno));
    return (p);
    }


    Best regards,

    --
    Kuang He
    Department of Physics
    University of Connecticut
    Storrs, CT 06269-3046

    Tel: +1.860.486.4919
    Web: http://www.phys.uconn.edu/~he/
    <pager-help-file.diff>
    _______________________________________________
    sac-dev mailing list
    sac-dev<at>iris.washington.edu
    http://www.iris.washington.edu/mailman/listinfo/sac-dev

    • Kuang He
      2008-09-14 19:32:50
      Dear Brian,

      On Sun, Sep 14, 2008 at 12:04 PM, Brian Savage <savage<at>uri.edu> wrote:
      Kuang He

      Looking over the patches I have a couple of suggestions/comments.

      Could we use ANSI C syntax rather than K&R for function declarations. This
      would allow for the easy creation of header files by copying and it is a
      current/standard notation.

      Sure, I actually prefer ANSI C syntax myself. I used K&R style
      function declarations just to conform to the existing code base. I'll
      use ANSI C style from now on.

      Pager use: We need to check the system for the less command on compilation.
      In fact we could check for less, then more, then fallback to the original
      code in sac.

      This is a good idea, but I'm not sure how to do it, so could you help
      me with this?

      A change like this might allow for syntax highlighting in the help files.

      Exactly, and we get to search within help files.

      This would mean the current paging code needs to be made into a
      seperate function. The pager value could always be checked for to override
      the default.

      I can try isolating the current paging code to a separate function.
      Where do you think I should put it, in src/ucf/wrhelp.c?


      Best regards,

      --
      Kuang He
      Department of Physics
      University of Connecticut
      Storrs, CT 06269-3046

      Tel: +1.860.486.4919
      Web: http://www.phys.uconn.edu/~he/

      • Brian Savage
        2008-09-14 20:00:15
        Kuang He,


        ANSI C it is. Unless someone else has any objections.

        To add in a check for "less" and or "more" see:
        Autoconf: Generic Program Checks
        http://www.gnu.org/software/autoconf/manual/autoconf-2.57/
        html_chapter/autoconf_5.html#SEC41
        Either one of these should work. You can give a list of programs to
        check for in the order you would like
        Macro: AC_CHECK_PROGS (variable, progs-to-check-for, [value-if-not-
        found], [path])
        Macro: AC_PATH_PROGS (variable, progs-to-check-for, [value-if-not-
        found], [path])
        This should go into the configure.in file, then the bootstrap needs
        to be re-run.

        Isolating the original pager code inside src/sac/wrhelp.c is probably
        the best place for it.

        Cheers
        Brian

        On Sep 14, 2008, at 12:32 PM, Kuang He wrote:

        Dear Brian,

        On Sun, Sep 14, 2008 at 12:04 PM, Brian Savage <savage<at>uri.edu> wrote:
        Kuang He

        Looking over the patches I have a couple of suggestions/comments.

        Could we use ANSI C syntax rather than K&R for function
        declarations. This
        would allow for the easy creation of header files by copying and
        it is a
        current/standard notation.

        Sure, I actually prefer ANSI C syntax myself. I used K&R style
        function declarations just to conform to the existing code base. I'll
        use ANSI C style from now on.

        Pager use: We need to check the system for the less command on
        compilation.
        In fact we could check for less, then more, then fallback to the
        original
        code in sac.

        This is a good idea, but I'm not sure how to do it, so could you help
        me with this?

        A change like this might allow for syntax highlighting in the help
        files.

        Exactly, and we get to search within help files.

        This would mean the current paging code needs to be made into a
        seperate function. The pager value could always be checked for to
        override
        the default.

        I can try isolating the current paging code to a separate function.
        Where do you think I should put it, in src/ucf/wrhelp.c?


        Best regards,

        --
        Kuang He
        Department of Physics
        University of Connecticut
        Storrs, CT 06269-3046

        Tel: +1.860.486.4919
        Web: http://www.phys.uconn.edu/~he/
        _______________________________________________
        sac-dev mailing list
        sac-dev<at>iris.washington.edu
        http://www.iris.washington.edu/mailman/listinfo/sac-dev


        • Kuang He
          2008-09-16 10:42:38
          Dear Brian,

          Thanks for the tip.

          How often do systems that run SAC come without ``more''? I guess it is
          rare. In this case, we can maybe use something like this

          AC_CHECK_PROGS(SAC_PAGER, [$PAGER less more cat])

          in configure.in, and determine which pager to use at runtime in this way:

          if environment variable $PAGER is defined and is not empty
          use that (user's preference has more priority)
          else
          use PAGER specified by configure script

          In the worst scenario users will end up using ``cat'', which means no
          paging at all -- this should hardly happen. In this case, users could
          still use something like Shift + PageUp/PageDn to scroll back and
          forth. The good thing about this approach is that we don't have to
          maintain the original pager code any more. What do you think?

          One the other hand, for users who install SAC from binary
          distributions, how do we decide which pager to use if they don't have
          their environment variable $PAGER set? I don't want them end up using
          ``cat'' or the original pager code, since many of them may already
          have ``less'' or ``more'' installed. Maybe after this feature is
          merged into the code, we can highly recommend them to set $PAGER in
          the manual?


          Best regards,

          --
          Kuang He
          Department of Physics
          University of Connecticut
          Storrs, CT 06269-3046

          Tel: +1.860.486.4919
          Web: http://www.phys.uconn.edu/~he/

          On Sun, Sep 14, 2008 at 1:00 PM, Brian Savage <savage<at>uri.edu> wrote:
          Kuang He,


          ANSI C it is. Unless someone else has any objections.

          To add in a check for "less" and or "more" see:
          Autoconf: Generic Program Checks
          http://www.gnu.org/software/autoconf/manual/autoconf-2.57/html_chapter/autoconf_5.html#SEC41
          Either one of these should work. You can give a list of programs to check
          for in the order you would like
          Macro: AC_CHECK_PROGS (variable, progs-to-check-for, [value-if-not-found],
          [path])
          Macro: AC_PATH_PROGS (variable, progs-to-check-for, [value-if-not-found],
          [path])
          This should go into the configure.in file, then the bootstrap needs to be
          re-run.

          Isolating the original pager code inside src/sac/wrhelp.c is probably the
          best place for it.

          • Brian Savage
            2008-09-16 19:13:53
            Kuang He

            Looks like less is available on Linux and OSX (the GNU version)
            I cannot find a version of less on Solaris, but Solaris does have the
            more command.
            I would propose doing a check like

            AC_CHECK_PROGS(SAC_PAGER, [ less more ])
            It would be nice to remove this code as the capability exists
            elsewhere outside of SAC.
            There is similar code in src/dfm/xlh.c to do paging for the listhdr
            command, but it is
            coupled to the output of the command. Same for the news command.
            Would you suggest
            that we change the way these commands operate as well ? This could
            be done with a
            pipe to the pager. It might be more trouble than it is worth. This
            also modifies the current behavior of SAC.

            I still think having a fallback to the original SAC paging code is
            the safest thing to do.

            Cheers
            Brian

            On Sep 16, 2008, at 3:42 AM , Kuang He wrote:

            Dear Brian,

            Thanks for the tip.

            How often do systems that run SAC come without ``more''? I guess it is
            rare. In this case, we can maybe use something like this

            AC_CHECK_PROGS(SAC_PAGER, [$PAGER less more cat])

            in configure.in, and determine which pager to use at runtime in
            this way:

            if environment variable $PAGER is defined and is not empty
            use that (user's preference has more priority)
            else
            use PAGER specified by configure script

            In the worst scenario users will end up using ``cat'', which means no
            paging at all -- this should hardly happen. In this case, users could
            still use something like Shift + PageUp/PageDn to scroll back and
            forth. The good thing about this approach is that we don't have to
            maintain the original pager code any more. What do you think?

            One the other hand, for users who install SAC from binary
            distributions, how do we decide which pager to use if they don't have
            their environment variable $PAGER set? I don't want them end up using
            ``cat'' or the original pager code, since many of them may already
            have ``less'' or ``more'' installed. Maybe after this feature is
            merged into the code, we can highly recommend them to set $PAGER in
            the manual?


            Best regards,

            --
            Kuang He
            Department of Physics
            University of Connecticut
            Storrs, CT 06269-3046

            Tel: +1.860.486.4919
            Web: http://www.phys.uconn.edu/~he/

            On Sun, Sep 14, 2008 at 1:00 PM, Brian Savage <savage<at>uri.edu> wrote:
            Kuang He,


            ANSI C it is. Unless someone else has any objections.

            To add in a check for "less" and or "more" see:
            Autoconf: Generic Program Checks
            http://www.gnu.org/software/autoconf/manual/autoconf-2.57/
            html_chapter/autoconf_5.html#SEC41
            Either one of these should work. You can give a list of programs
            to check
            for in the order you would like
            Macro: AC_CHECK_PROGS (variable, progs-to-check-for, [value-if-not-
            found],
            [path])
            Macro: AC_PATH_PROGS (variable, progs-to-check-for, [value-if-not-
            found],
            [path])
            This should go into the configure.in file, then the bootstrap
            needs to be
            re-run.

            Isolating the original pager code inside src/sac/wrhelp.c is
            probably the
            best place for it.
            _______________________________________________
            sac-dev mailing list
            sac-dev<at>iris.washington.edu
            http://www.iris.washington.edu/mailman/listinfo/sac-dev



            • Kuang He
              2008-09-17 00:22:38
              Dear Brian,

              On Tue, Sep 16, 2008 at 12:13 PM, Brian Savage <savage<at>uri.edu> wrote:
              There is similar code in src/dfm/xlh.c to do paging for the listhdr command,
              but it is
              coupled to the output of the command. Same for the news command. Would you
              suggest
              that we change the way these commands operate as well ? This could be done
              with a
              pipe to the pager. It might be more trouble than it is worth. This also
              modifies the current behavior of SAC.

              I agree with you. For the time being, maybe we can just leave the
              paging part of the listhdr command alone.

              Looks like less is available on Linux and OSX (the GNU version)
              I cannot find a version of less on Solaris, but Solaris does have the more
              command.
              I would propose doing a check like
              AC_CHECK_PROGS(SAC_PAGER, [ less more ])
              It would be nice to remove this code as the capability exists elsewhere
              outside of SAC.

              I still think having a fallback to the original SAC paging code is the
              safest thing to do.

              There are some problems with checking for available pager programs
              during the ./configure stage:

              (1) For example, if the user does not have ``less'' during the
              ./configure stage, SAC will use ``more'' to do the paging. After SAC
              is compiled and installed, if the users installs ``less'' but does not
              set the environment variable $PAGER, SAC will still be using ``more''
              all the time. Yes, it is the user's responsibility to set $PAGER
              correctly, but who knows.

              (2) If developer A compiles a binary SAC package on a system that has
              ``less'' installed and distributes it to user B who does not have
              ``less'' on her system, SAC will still try to use ``less'' as its
              pager, which will result in a failure.

              To solve this problem, I propose that we check for pager programs at
              runtime, following the practice of ``man''. So when SAC starts, it'll
              run through this decision process:

              if the environment variable $PAGER is set to a non-empty value {
              use it
              } else {
              SAC will look for executable files at these locations one by one
              /usr/bin/less
              /bin/less
              /usr/bin/more
              /bin/more
              and will use the first one available.
              If none of the above pages are available, SAC will fall back to its
              original paging code.
              }

              Maybe we could put the above in the initialization part of SAC. How
              does that sound?


              Best regards,

              --
              Kuang He
              Department of Physics
              University of Connecticut
              Storrs, CT 06269-3046

              Tel: +1.860.486.4919
              Web: http://www.phys.uconn.edu/~he/

              • Kuang He
                2008-09-17 05:52:31
                On Tue, Sep 16, 2008 at 5:22 PM, Kuang He <icrazy<at>gmail.com> wrote:
                To solve this problem, I propose that we check for pager programs at
                runtime, following the practice of ``man''. So when SAC starts, it'll
                run through this decision process:

                if the environment variable $PAGER is set to a non-empty value {
                use it
                } else {
                SAC will look for executable files at these locations one by one
                /usr/bin/less
                /bin/less
                /usr/bin/more
                /bin/more
                and will use the first one available.
                If none of the above pages are available, SAC will fall back to its
                original paging code.
                }

                I've implemented the above algorithm, and the patch (against v101.2)
                is attached.

                If you have applied the previous patch included in:
                http://www.iris.washington.edu/pipermail/sac-dev/2008-September/000100.html

                You need to reverse that patch first using ``patch -R'', before
                applying this patch.

                In this patch:

                - The original paging code has been isolated to internal_pager(), in
                file src/ucf/wrhelp.c

                - SAC will try to determine which pager to use and whether to use the
                internal pager or not during the initialization stage

                - When the user has a broken setting of $PAGER, the error message is clearer:

                $ export PAGER=xxxx
                $ sac
                SAC> h
                sh: xxxx: not found
                Error loading pager xxxx!

                - A global variable pager has been defined in src/top/initsac.c, and I
                don't know whether there is a better place for that.


                Best regards,

                --
                Kuang He
                Department of Physics
                University of Connecticut
                Storrs, CT 06269-3046

                Tel: +1.860.486.4919
                Web: http://www.phys.uconn.edu/~he/

                Attachments
22:07:56 v.eb79165e