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 PDF outline support #134

Merged
merged 1 commit into from
Jan 28, 2019
Merged

add PDF outline support #134

merged 1 commit into from
Jan 28, 2019

Conversation

matsud224
Copy link
Contributor

@matsud224 matsud224 commented Dec 12, 2018

This PR adds PDF outline support(also called "Bookmark").

This PR requires some camlpdf extensions of gfngfn/camlpdf#2. (So, CI tests may fail.)

Added primitives

  • register-outline : (int * string * string * bool) list -> unit
    This primitive registers a document outline. Given quartet contains depth of hierarchy, text, label of named destination and default open state.
  • extract-string : inline-boxes -> string
    This primitive extracts string from inline-boxes. Some contents like a math command are ignored. This primitive is used to get a section title as string.

Changed packages

  • +chapter +section +subsection in stdja stdjabook stdjareport
    2nd optional argument outline-titile-opt : string? is added to these commands. If this optional argument is specified, that string is used as an alternative title for PDF outline.

@gfngfn
Copy link
Owner

gfngfn commented Dec 14, 2018

Thank you for developing such an attractive feature as always! I confirmed for now that it works as intended. Maybe I will have, however, a little question about whether the newly added primitives are natural and convenient as an API, when I look into your implementations (especially those of packages). I would appreciate it if you could respond to some discussion about it.

@gfngfn
Copy link
Owner

gfngfn commented Jan 28, 2019

I am so sorry for the late response. I understand what you have implemented to provide functionalities for outputting PDF outlines, and it seems quite well-written to me. Although I am not reluctant to merge this PR, I have some comments as to what interface we should provide for users:

  1. IMHO, it may be more natural in the long run to provide register-outline as a primitive of type ((string * string * bool) tree) list -> unit by using a newly introduced algebraic data type 'a tree defined by

    type 'a tree = Node of 'a * ('a tree) list
    

    than as that of type (int * string * string * bool) list -> unit. This is because the latter can receive a list that does not exactly represent a tree (e.g. [(0, foo, foo, true); (2, bar, bar, true)]).

  2. To be honest, I am slightly hesitant to introduce extract-string : inline-boxes -> string because SATySFi has kept so far the principle that, once texts are converted into box rows, they cannot be reverted. This is not just based on an aethetic viewpoint but also on the fact that such a reversion simply forgets skips, glues, graphics, etc. that are embedded in box rows.

In spite of these concerns, it seems quite beneficial for practical use to introduce the primitives as you suggest, and thus I will merge your PR for now. (To remedy the concerns above, the API may be redesigned nonetheless when the version is upgraded to v0.1.0 and the backward compatibility is discarded.) Thank you very much for the improvement.

@gfngfn
Copy link
Owner

gfngfn commented Jan 28, 2019

Oh I forgot to mention a slight modification I will make. The definition of extract_string in horzBox.ml will be fixed; see ddcf8f8.

@gfngfn gfngfn merged commit 4d6d4e2 into gfngfn:master Jan 28, 2019
@matsud224 matsud224 deleted the dev-outline branch February 6, 2019 03:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants