Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add pphtml tool #108

Open
windymilla opened this issue Jan 25, 2024 · 4 comments
Open

Add pphtml tool #108

windymilla opened this issue Jan 25, 2024 · 4 comments
Labels
core feature Required for basic PPing

Comments

@windymilla
Copy link
Collaborator

Just putting a few notes here about pphtml to aid us in deciding what to do and how to link pphtml with Guiguts. Just thoughts that I didn't want to lose, rather than definite suggestions of things to do:

  1. Current version of pphtml.py runs on the command line and can redirect output to a file.
  2. Presumably a similar technique to that used for pptxt could be used, with StringIO objects passed to a pphtml function, and a small "main" section so it can continue to be run as a standalone script
  3. Current version of pphtml puts some HTML in its output. The new version of pptxt either outputs plain text or decorated text (colored using ANSI terminal codes) but not HTML. It would be good for the revised pphtml to be able to do the same, i.e. HTML, plain or decorated.
  4. If we are aiming to make the output of the two tools consistent with one another, and potentially with other tools yet to be written, like jeebies, should we consider whether any of the code in the tools ought to be common, particularly to support the 2/3 ways the output is formatted. Maybe something for the future - perhaps pphtml, pptxt, jeebies,... could/should live in a repo together with some shared routines, though I guess that would complicate their use as standalone scripts.
@cpeel
Copy link
Member

cpeel commented Jan 25, 2024

Looking at pphtml.py there are a few changes we'll want to make to use it as a module where we can pass in data.

  • We need to remove all exit()s and have it raise exceptions that can be caught.
  • It uses the file command to detect the character set (see charsetCheck()) which won't work on a buffer and won't work on WIndows.
  • It uses the input file's path to iterate over all images (self.sdir) so we'll want to pass that into the module instead.
  • I think to support non-html output we just need to change the saveReport() post-processing. I'll give some thought about if/how we can standardize across tools

Since this is an existing tool we probably want to do some small iterative PRs with these improvements so we can test them individually. Thumbing through it I saw some other improvements we could make which also means we probably need some meager samples to test against to detect regressions along the way.

@windymilla windymilla added the core feature Required for basic PPing label Feb 18, 2024
@windymilla
Copy link
Collaborator Author

Given the successful implementation of the functionality of pptxt in GG2, I think it will end up being best to do pphtml in a similar way.

@windymilla
Copy link
Collaborator Author

Suggestion: include the functionality of the ppvimage in the new pphtml.

@tangledhelix
Copy link
Collaborator

As noted in this forum thread, pphtml attempts to check for unprocessed ppgen commands in the HTML. I think it's a bit too naive as-is. It just checks for a <p> tag followed by a dot, and therefore it is triggered by a paragraph which begins with an ellipsis.

if ( $line =~ /<p>\./ ) {
    printf LOGFILE ( "%d:0 Possible PPG command: %s\n", $count, $line );
}

In the reimplementation it should be improved, I see one of two ways to do it:

  1. The specific-exclusion route: add an and to the condition such that <p>\.\.\. doesn't match this check.
  2. Or make it more aware of ppgen's command syntax: all ppgen commands should match <p>\.[a-z]{2} (a period followed by two lowercase letters)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core feature Required for basic PPing
Projects
None yet
Development

No branches or pull requests

3 participants