- removed comment
piraha breaks ThornDoc building
Doing make ThornDocHTML
I get an error message in eg in./repos/flesh/doc/HTML/ThornDoc/CactusConnect/Socket/LOG_SCHEDLATEX_MSGS of
Undefined subroutine &piraha::parse_peg_file called at /home/rhaas/postdoc/gr/cactus/ET_trunk/lib/sbin/ScheduleParser.pl line 83.
this seems to be caused by the Piraha perl module not being loaded in those files.
Keyword:
Comments (17)
-
reporter -
reporter - removed comment
This fixes it:
diff --git a/lib/sbin/InterLatex.pl b/lib/sbin/InterLatex.pl index e4b0a460..898b3ed4 100755 --- a/lib/sbin/InterLatex.pl +++ b/lib/sbin/InterLatex.pl @@ -56,6 +56,7 @@ my $sbin_dir = "${cctk_home}lib/sbin"; require "$sbin_dir/ThornUtils.pm"; # for use of create_parametere_database +require "$sbin_dir/Piraha.pm"; require "$sbin_dir/interface_parser.pl"; require "$sbin_dir/CSTUtils.pl"; diff --git a/lib/sbin/ParamLatex.pl b/lib/sbin/ParamLatex.pl index 06f1b011..b1810fdd 100644 --- a/lib/sbin/ParamLatex.pl +++ b/lib/sbin/ParamLatex.pl @@ -62,6 +62,7 @@ my $sbin_dir = "${cctk_home}lib/sbin"; require "$sbin_dir/ThornUtils.pm"; # for use of create_parametere_database +require "$sbin_dir/Piraha.pm"; require "$sbin_dir/parameter_parser.pl"; require "$sbin_dir/CSTUtils.pl"; diff --git a/lib/sbin/SchedLatex.pl b/lib/sbin/SchedLatex.pl index f8b52700..4e1db1f9 100755 --- a/lib/sbin/SchedLatex.pl +++ b/lib/sbin/SchedLatex.pl @@ -53,6 +53,7 @@ if ($h || $help) { $cctk_home .= '/' if (($cctk_home !~ /\/$/) && defined $cctk_home); my $sbin_dir = "${cctk_home}lib/sbin"; +require "$sbin_dir/Piraha.pm"; require "$sbin_dir/ScheduleParser.pl"; require "$sbin_dir/CSTUtils.pl"; diff --git a/lib/sbin/parameter_parser.pl b/lib/sbin/parameter_parser.pl index cb39d013..3cdbf9fc 100644 --- a/lib/sbin/parameter_parser.pl +++ b/lib/sbin/parameter_parser.pl @@ -16,6 +16,13 @@ use strict; use Carp; my $ccl_file; +sub trim_quotes +{ + my $str = shift; + $str =~ s/^(["'])(.*)\1$/$2/s; + return $str; +} + #/*@@ # @routine create_parameter_database # @date Wed Sep 16 11:45:18 1998
however it does not seem to be the "right" solution. Shouldn't require piraha be in the actual parsers? Similarly parameter parser uses a function (trim_spaces) that was originally only defined in interface parser and which has no GRdoc text whatsoever.
-
reporter - changed status to open
- assigned issue to
- removed comment
-
reporter - changed status to open
- removed comment
-
- removed comment
So I think including Piraha the way you have it is correct, if you think of Piraha as being "like" CSTUtils, a utility that all the parsers need.
The trim_quotes() function should probabaly move to CSTUtils (and get GRDoc comments). Do you agree?
-
reporter - removed comment
Hello Steve,
well I think I am not including it properly. Right now in both CST and the LaTeX parsers, the parsers require that their caller already provides piraha for them. This seems wrong since the caller usually does not even user piraha and also violates encapsulation of internals since the caller should not know how the parsers internally parse the files.
Moving trim_quotes() into a common library package is of course fine with me, as long as all packages that use trim_quotes() actually require the library package. I should also receive a GRdoc header if possible.
-
- removed comment
Well, I could include Piraha in each of the parsers. It would have to look like this:
and rely on the calling routine to set $main::sbin_dir correctly. Alternatively, we could put "use lib $sbin_dir" in the CST and "use Piraha;" in each of the parsers. Or we could include Piraha in the CSTUtils? Which do you prefer? I put Piraha in the CST because it felt more like the style of what had been done to me.
-
reporter - removed comment
My feeling would be that the modules (ie the parsers) should encapsulate information that CST or their other callers do not need to know about (such as whether Piraha or another parser is used) and that as little information as possible should be passed from callers to the modules using global variables. So with that in mind I would move both the
require "$main::sbin_dir/Piraha.pm";
and the code that computes sbin_dir to the parser packages or (alternatively and probably better) have main adjust the perl search path to include the Cactus perl module directory where Piraha lives. -
- removed comment
OK, so how about we put "use lib $sbin_dir;" in CST, and "use Piraha;" in the parsers?
-
reporter - removed comment
See: https://bitbucket.org/cactuscode/cactus/pull-requests/36/add-use-piraha-to-parsers/diff
Would this be what you had in mind? It lets me build all the html docs and compile cactus as well.
-
- removed comment
Yes. I thought this was something you were asking me to do. :) Does that mean we should close this ticket?
-
reporter - removed comment
If you review the pull request and give me a "Please apply" I will be happy to do so. Note that while I did indeed read the Camel book on perl this was many years in the past (say in 2001 or so) and by now most of my perl knowledge is obtained from the Internet so I think I got this all working and the methods seem to be ok, if there is a simpler (rather than just "more than one") way to achieve that I'd be happy to implement that as well. Naturally if this is "fine" as is, then I am also very happy not to spend more time on this than needed.
-
- changed status to open
- removed comment
Looks good to me. Please apply.
-
reporter - changed status to resolved
- removed comment
Applied as git hash 5dd70e1e of the flesh.
-
reporter - changed status to open
- removed comment
-
reporter - changed status to resolved
- removed comment
Hopefully now fixed in 96d510ebcdc0ff5cc281e4f33c5321480967545c "Cactus: properly set up cctk_bin dir in documentation parsers".
-
reporter - edited description
- changed status to closed
- Log in to comment