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

floating marginpar #2068

Merged
merged 4 commits into from
Aug 4, 2023
Merged

floating marginpar #2068

merged 4 commits into from
Aug 4, 2023

Conversation

teepeemm
Copy link
Contributor

This allows marginpars to float (in the CSS sense), so that they display in order instead of overlapping, fixing #2041. LaTeXML-marginpar.css doesn't appear to mesh well with LaTeXML-navbar-left.css (for example .ltx_page_main { width } is determined by which is loaded last), so I've not tried to resolve that. On the other hand, if someone is loading both, then they will probably want their own css file anyway.

The tex code I was using to test this out:

\documentclass{article}
\usepackage{latexml}
\begin{lxNavbar}
\lxContextTOC
\end{lxNavbar}
\begin{document}
\section{Section Title}
A small amount of text.\marginpar{first}\marginpar{second second second second}
A large amount of text. A large amount of text. A large amount of text. A large amount of text. A large amount of text. A large amount of text. A large amount of text. A large amount of text. A large amount of text. A large amount of text. A large amount of text. A large amount of text. A large amount of text. A large amount of text. A large amount of text. A large amount of text. A large amount of text. A large amount of text. A large amount of text. A large amount of text. A large amount of text. A large amount of text. A large amount of text. A large amount of text. A large amount of text. A large amount of text. A large amount of text.
A large amount of text.\marginpar{third}\marginpar{fourth}
A small amount of text.
\end{document}

@teepeemm teepeemm marked this pull request as draft June 5, 2023 22:08
@teepeemm teepeemm marked this pull request as ready for review June 6, 2023 22:00
@teepeemm
Copy link
Contributor Author

teepeemm commented Jun 6, 2023

I've updated this to account for the optional argument to marginpar. If it's present, and \reversemarginpar has been issued, then that will be used. And if \reversemarginpar has been issued and LaTeXML-marginpar.css is being used, then the marginpar will appear on the left side instead of the right. I've also updated that css to provide the proper widths depending on which types of marginpars are present (none, left, right, or both). And by adjusting the size of .ltx_page_content, it also works if one of the navbar.css is present.

Quirks:

  • If the document has been --split, the .ltx_page_content { width } will vary depending on which types of marginpars are present on a particular page. If we want to make that not happen, I think we would need to do something like DefRewrite(xpath=>'/document',attributes=>{class=>'ltx_has_marginpar_normal'});. Is that correct? Should we go that route?
  • An empty argument for \marginpar{} causes the empty box to appear in LaTeXML. LaTeX typesets this as well (I think), but there's no box so you can't tell it happened. Should I add a test that the marginpar is typesetting some content, and do nothing if there's no content? On the other hand, I can't think of a good reason to be typesetting empty marginpars.

The tex code I was using to test this out:

\documentclass{article}
\usepackage{latexml}
\newcommand{\sentence}{A long line of text to take up some space.}
\newcommand{\longline}{\sentence\ \sentence\ \sentence\ \sentence\ \sentence\ \sentence}
\begin{lxNavbar}
\lxContextTOC
\end{lxNavbar}
\begin{document}
\reversemarginpar % reversing
\section{Section Title}
A small amount of text.\marginpar{first}\marginpar{second second second second}
\longline\ \longline\ \longline\marginpar{third}\marginpar{fourth}
A small amount of text.

\longline\marginpar[show left]{hide right}

\longline\marginpar{show only on left}

\longline\marginpar[]{hide non empty right}

\normalmarginpar % unreversing
\longline\marginpar{show only on right}

\longline\marginpar[hide left]{show right}

\longline\marginpar[hide non empty left]{}

\section{Section Title}
A section with no margin pars.
\end{document}

@teepeemm
Copy link
Contributor Author

Grr.

The real kicker is that I can't commit the actual changes because latexmllint says

lib/LaTeXML/Package/LaTeX.pool.ltxml  has syntax issues
  Bareword "CC_ESCAPE" not allowed while "strict subs" in use at LaTeXML/blib/lib/LaTeXML/Core/State.pm line 108.
  Bareword "CC_BEGIN" not allowed while "strict subs" in use at LaTeXML/blib/lib/LaTeXML/Core/State.pm line 108.
  Bareword "CC_END" not allowed while "strict subs" in use at LaTeXML/blib/lib/LaTeXML/Core/State.pm line 108.
  Bareword "CC_MATH" not allowed while "strict subs" in use at LaTeXML/blib/lib/LaTeXML/Core/State.pm line 108.
  Bareword "CC_ALIGN" not allowed while "strict subs" in use at LaTeXML/blib/lib/LaTeXML/Core/State.pm line 108.
  Bareword "CC_EOL" not allowed while "strict subs" in use at LaTeXML/blib/lib/LaTeXML/Core/State.pm line 108.
  Bareword "CC_PARAM" not allowed while "strict subs" in use at LaTeXML/blib/lib/LaTeXML/Core/State.pm line 108.
  Bareword "CC_SUPER" not allowed while "strict subs" in use at LaTeXML/blib/lib/LaTeXML/Core/State.pm line 108.
  Bareword "CC_SUB" not allowed while "strict subs" in use at LaTeXML/blib/lib/LaTeXML/Core/State.pm line 108.
  Bareword "CC_SPACE" not allowed while "strict subs" in use at LaTeXML/blib/lib/LaTeXML/Core/State.pm line 108.
  Bareword "CC_SPACE" not allowed while "strict subs" in use at LaTeXML/blib/lib/LaTeXML/Core/State.pm line 108.
  Bareword "CC_COMMENT" not allowed while "strict subs" in use at LaTeXML/blib/lib/LaTeXML/Core/State.pm line 108.
  Bareword "CC_ACTIVE" not allowed while "strict subs" in use at LaTeXML/blib/lib/LaTeXML/Core/State.pm line 108.
  Bareword "CC_ESCAPE" not allowed while "strict subs" in use at LaTeXML/blib/lib/LaTeXML/Core/State.pm line 108.
  Bareword "CC_ACTIVE" not allowed while "strict subs" in use at LaTeXML/blib/lib/LaTeXML/Core/State.pm line 108.
  Bareword "CC_LETTER" not allowed while "strict subs" in use at LaTeXML/blib/lib/LaTeXML/Core/State.pm line 114.
  Bareword "CC_LETTER" not allowed while "strict subs" in use at LaTeXML/blib/lib/LaTeXML/Core/State.pm line 115.
  Bareword "CC_LETTER" not allowed while "strict subs" in use at LaTeXML/blib/lib/LaTeXML/Core/State.pm line 118.

but I've not touched State.pm, so I'm not sure why that happened. Any idea what's going on? Any idea how I can fix the git commits to be more reasonable?

@dginev
Copy link
Collaborator

dginev commented Jul 22, 2023

@teepeemm Ah this is unfortunate, this branch has diverged. I generally never use git merge exactly since it can be quite treacherous when it misfires - I get better mileage out ofrebase.

You can branch off from master and cherry-pick individual commits from here, making a new PR, or use this branch by rewinding back to before your first git merge. (git checkout to the right commit, then a git push --force would yank out the merges and anything else that followed).

I always do a git fetch upstream; git rebase upstream/master myself, resolving any potential conflicts and then git push --force origin mybranch to update the PR.

@dginev
Copy link
Collaborator

dginev commented Jul 22, 2023

Btw @brucemiller , it looks like latexmllint is indeed failing on the master branch, as Tim reports here.

I am not sure how to convince perltidy to recognize constants, currently it complains:

$ perl tools/latexmllint lib/LaTeXML/Core/State.pm

lib/LaTeXML/Core/State.pm  has syntax issues
  Bareword "CC_ESCAPE" not allowed while "strict subs" in use at /home/deyan/git/my-LaTeXML/tools/../blib/lib/LaTeXML/Core/State.pm line 108.
  Bareword "CC_BEGIN" not allowed while "strict subs" in use at /home/deyan/git/my-LaTeXML/tools/../blib/lib/LaTeXML/Core/State.pm line 108.
  Bareword "CC_END" not allowed while "strict subs" in use at /home/deyan/git/my-LaTeXML/tools/../blib/lib/LaTeXML/Core/State.pm line 108.
  Bareword "CC_MATH" not allowed while "strict subs" in use at /home/deyan/git/my-LaTeXML/tools/../blib/lib/LaTeXML/Core/State.pm line 108.
  Bareword "CC_ALIGN" not allowed while "strict subs" in use at /home/deyan/git/my-LaTeXML/tools/../blib/lib/LaTeXML/Core/State.pm line 108.
  Bareword "CC_EOL" not allowed while "strict subs" in use at /home/deyan/git/my-LaTeXML/tools/../blib/lib/LaTeXML/Core/State.pm line 108.
  Bareword "CC_PARAM" not allowed while "strict subs" in use at /home/deyan/git/my-LaTeXML/tools/../blib/lib/LaTeXML/Core/State.pm line 108.
  Bareword "CC_SUPER" not allowed while "strict subs" in use at /home/deyan/git/my-LaTeXML/tools/../blib/lib/LaTeXML/Core/State.pm line 108.
  Bareword "CC_SUB" not allowed while "strict subs" in use at /home/deyan/git/my-LaTeXML/tools/../blib/lib/LaTeXML/Core/State.pm line 108.
  Bareword "CC_SPACE" not allowed while "strict subs" in use at /home/deyan/git/my-LaTeXML/tools/../blib/lib/LaTeXML/Core/State.pm line 108.
  Bareword "CC_SPACE" not allowed while "strict subs" in use at /home/deyan/git/my-LaTeXML/tools/../blib/lib/LaTeXML/Core/State.pm line 108.
  Bareword "CC_COMMENT" not allowed while "strict subs" in use at /home/deyan/git/my-LaTeXML/tools/../blib/lib/LaTeXML/Core/State.pm line 108.
  Bareword "CC_ACTIVE" not allowed while "strict subs" in use at /home/deyan/git/my-LaTeXML/tools/../blib/lib/LaTeXML/Core/State.pm line 108.
  Bareword "CC_ESCAPE" not allowed while "strict subs" in use at /home/deyan/git/my-LaTeXML/tools/../blib/lib/LaTeXML/Core/State.pm line 108.
  Bareword "CC_ACTIVE" not allowed while "strict subs" in use at /home/deyan/git/my-LaTeXML/tools/../blib/lib/LaTeXML/Core/State.pm line 108.
  Bareword "CC_LETTER" not allowed while "strict subs" in use at /home/deyan/git/my-LaTeXML/tools/../blib/lib/LaTeXML/Core/State.pm line 114.
  Bareword "CC_LETTER" not allowed while "strict subs" in use at /home/deyan/git/my-LaTeXML/tools/../blib/lib/LaTeXML/Core/State.pm line 115.
  Bareword "CC_LETTER" not allowed while "strict subs" in use at /home/deyan/git/my-LaTeXML/tools/../blib/lib/LaTeXML/Core/State.pm line 118.
  Compilation failed in require at /home/deyan/git/my-LaTeXML/tools/../blib/lib/LaTeXML/Core/Token.pm line 22.
  BEGIN failed--compilation aborted at /home/deyan/git/my-LaTeXML/tools/../blib/lib/LaTeXML/Core/Token.pm line 22.
  Compilation failed in require at lib/LaTeXML/Core/State.pm line 17.
  BEGIN failed--compilation aborted at lib/LaTeXML/Core/State.pm line 17.
lib/LaTeXML/Core/State.pm fails commit criterion

@dginev
Copy link
Collaborator

dginev commented Jul 22, 2023

As a qucik experiment, I moved the catcode constants to a new LaTeXML::Core::Catcode which avoids the circular dependencies (between State.pm and Token.pm), and perltidy seemed happier. Something to consider, @brucemiller ?

say in LaTeXML/Core/Catcode.pm

use base qw(Exporter);
our @EXPORT = (
  # Catcode constants
  qw( CC_ESCAPE  CC_BEGIN  CC_END     CC_MATH
    CC_ALIGN   CC_EOL    CC_PARAM   CC_SUPER
    CC_SUB     CC_IGNORE CC_SPACE   CC_LETTER
    CC_OTHER   CC_ACTIVE CC_COMMENT CC_INVALID
    CC_CS      CC_MARKER CC_ARG)
);

use constant CC_ESCAPE  => 0;
use constant CC_BEGIN   => 1;
use constant CC_END     => 2;
use constant CC_MATH    => 3;
use constant CC_ALIGN   => 4;
use constant CC_EOL     => 5;
use constant CC_PARAM   => 6;
use constant CC_SUPER   => 7;
use constant CC_SUB     => 8;
use constant CC_IGNORE  => 9;
use constant CC_SPACE   => 10;
use constant CC_LETTER  => 11;
use constant CC_OTHER   => 12;
use constant CC_ACTIVE  => 13;
use constant CC_COMMENT => 14;
use constant CC_INVALID => 15;
# Extended Catcodes for expanded output.
use constant CC_CS     => 16;
use constant CC_MARKER => 17;    # non TeX extension!
use constant CC_ARG    => 18;    # "out_param" in B Book

@teepeemm teepeemm force-pushed the marginpar branch 2 times, most recently from 85a7131 to 937881a Compare July 24, 2023 17:31
@teepeemm
Copy link
Contributor Author

@dginev I think I managed to resolve the conversations and get the commit history back to looking reasonable, so that this is ready to review and merge. Let me know if I need to do anything else.

@dginev
Copy link
Collaborator

dginev commented Jul 24, 2023

Nice, thank you @teepeemm .

Hm, so I made a quick exploration into how the optional argument of \marginpar is used, and I wonder if we aren't creating a mildly misleading feature by respecting it. Generally, latexml does not keep page counts, and does not have a reliable distinction between being on an "even" or "odd" page (unless I am missing something new?). As an example, in the PDF you would expect this simple \marginpar call to use its different arguments on even/odd pages:

\documentclass{book}
\def\greeting{\marginpar[even greetings]{odd greetings}}
\begin{document}

\greeting
\clearpage
\greeting

\end{document}

I wonder if the Perl binding should only ever use the mandatory (odd page) argument for marginpar, to avoid ever preserving a note expected for the left margin in the XML.
As my usual example, a site such as ar5iv only ever uses the right margin (of a sufficiently wide viewport) for showing notes, so in the \marginpar sense all pages appear to be "odd" there.

What do you think?

@dginev
Copy link
Collaborator

dginev commented Jul 24, 2023

I now also reread the comments, and I understood that you're interested in a left-margin CSS variant, additionally activated via \reversemarginpar, which will have to be done globally for all marginpar notes (since we can't alternate reliably). That kind of sounds sensible...

In that case, I may want to deactivate \reversemarginpar to \relax in the current arXiv conversions, since there wouldn't be a reasonable way to emulate the page counts for legacy documents and one will keep getting left marks in the right margin...

I'm still tempted by the simplicity of ignoring the optional argument and treating all pages as odd for \marginpar purposes, but I may be in the minority.

@teepeemm
Copy link
Contributor Author

Why can't we alternate left and right marginpar reliably? Is there an arXiv document that uses \reversemarginpar that we could experiment with? My guess is that \reversemarginpar isn't used all that often, so deactivation to \relax may not be necessary.

@dginev
Copy link
Collaborator

dginev commented Jul 24, 2023

Why can't we alternate left and right marginpar reliably?

We don't even have the concept of pages in HTML, but we kind of do in ePub. Seen differently - say we emit an ePub and show it on a fixed page view of an eink reader. The concrete page numbers will depend on the resolution and size of the display. So a margin note which appears at the top of page 2k+1 on a 10 inch screen may be at the bottom of page 2q on a 7 inch screen.

Is there an arXiv document that uses \reversemarginpar that we could experiment with?

I can try to find one, but the server hosting the documents is down for maintenance this week. Will dig through my local copy...

Edit: what the latex docs call "two-sided layout" is generally outside of what we've been working on in our conversions to the web paradigm.

@brucemiller
Copy link
Owner

I don't think we've quite captured the intention of \marginpar, and are maybe drifting away from it. The basic idea is to put a note in the "outside" margin, opposite the usually narrower gutter where the pages are sewn or between columns. No counter or dagger marker is needed, since the note should appear directly alongside the place the note was introduced. The optional argument allows you to provide an alternate text for when the note appears on the left (for whatever reason that it appears there), in case there is some directionality in the text (eg. an arrow pointing to the text?). The \reversemarginpar says to place the note on the "inner" margin (except double column); it doesn't (directly) choose which note to use. I'd expect both the optional argument and reversing to be relatively rare, used only for production page design & layout.

Understanding that, we can now ask how that should be carried over to XML/HTML, where we don't (necessarily) have pages, columns, or even margins --- but we might. The lowest level would be latexml's original behavior to synthesize a marker (dagger) and have the note appear as a popup. The next level would be to build-in a wide margin, either on left or right and place the notes in that margin, without marker, alongside its source (choosing the optional argument as appropriate). The \reversemarginpar would be relevant at most for choosing which side you're creating the wide margin.

For a truly responsive solution, however, you may have variable margins, may have multiple columns, and you won't know till you detect the device & the user resizes the window. So the XML really should convert & to store both versions of the note, with appropriate attributes/classes, and defer the marker, display & positioning as late as possible to CSS (& javascript if needed). In this scenario the optional argument would be used, but \reversemarginpar is not really meaningful.

@teepeemm
Copy link
Contributor Author

I'm not seeing a purely CSS way to determine if a marginpar has appeared in the left or right margin, so I think it would need to involve Javascript for the truly responsive solution.

I think this PR does accomplish the "next level" rendering.

doc.sty (often used for typesetting tex documentation) does use \reversemarginpar and \marginpar{term} (along with a much wider left margin). But I do agree that reversing is in general rare, and switching back and forth within a document especially so.

@brucemiller
Copy link
Owner

I'm just not so clear that this is actually an improvement (yet); \reversemarginpar seems incorrect; it's not used to select the right or left form of the note, it's used to select whether notes go on the inside or outside. That may seem like the same thing if the default is to put the note in the right margin, but it isn't quite. Consider the margin note:

\marginnote[TaDah $\rightarrow$]{$leftarrow$ TaDah}

The first arg should never appear in the right margin, the 2nd arg should never appear in the left margin.

So, I'm thinking if it's worth handling the optional arg at all, we should be storing two notes, with appropriate roles or classes and using CSS; for purpose of argument role=margin-left|margin-right. I think the point with CSS is not to determine if a specific note appeared in the left or right, but rather to use CSS (and media queries, etc) to decide on the margin layout, and then to decide which versions of the notes will appear at all, and where. Thus if you're doing notes in the left margin, you'd be activating the margin-left and generic margin, while the margin-right would get display none.

@teepeemm
Copy link
Contributor Author

But in html, we don't have pages, so everything acts like page 1, and outside <=> right and inside <=> left. An ePub will have left and right pages, but then we need to figure out if we have a left page or a right page, which doesn't appear to be doable within CSS.

Without ePub and some reliable way to determine if it's a left or right page, then we can assume we have a right page. And if we know that we have a right page in the output, then when converting to XML, we know which argument we want to use, and don't need to keep the other.

@dginev
Copy link
Collaborator

dginev commented Jul 26, 2023

But in html, we don't have pages, so everything acts like page 1

That's if you are doing a right-margin CSS design. If you do a left-margin CSS design, everything acts like page 2 :>

A problem that hasn't gotten enough attention is legacy authored documents. In arXiv, an author may have known for certain that a specific \marginpar will appear on an even page, and written it as \marginpar[Important remark $\rightarrow$]{}. For that case, irrespective of which CSS theme is used, the only usable argument is the optional #1. Similarly in the inverse case of \marginpar[]{$\leftarrow$ Important remark}.

With that limited application in mind, I'd be happy with a solution that uses arg #2 if non-empty, otherwise #1 if non-empty, or do nothing if both are left empty. And to ignore all other subtleties needed for "two-sided layout" in PDF, leaving us with a single note to style in the HTML. Of course we could open a large project working on "two-sided layout" in HTML+CSS, but I'm not sure it's the right time to start on such an adventure...

@brucemiller
Copy link
Owner

The point is to create a passably nice document by default, while preserving enough information and semantics to give people the flexibility to adapt the styling and layout for whatever medium they're using; webpages, cellphones, epub, etc. So while a default assumption to make the entire document look like a right-hand page is perfectly reasonable, to build in the assumptions that that's the only thing you'll ever be able to do is not ok.

Folks ought to be able to easily create a CSS layout with the margin notes on the right or the left. As you point out left notes likely will interfere with a left navbar; so let people choose how it should be laid out. On that basis, I'd be more inclined to think we should ignore \reversemarginpar entirely, since it really doesn't apply in most cases, or should be left to the top-level design choices.

In any case, the class ltx_marginpar_reverse is misnamed & confusing as you've interpreted it; you're using it to mark left marginpars.

While you're probably right that you can't determine whether the note is on the left or right from CSS, you've got the process backwards: it's the CSS that will be applying the styling and layout that makes it be a left or right hand page (eg. unsymmetric margins, or whatever); so the CSS can easily display or hide whichever notes it wants based on the same choices.

@brucemiller
Copy link
Owner

@dginev tells me I wasn't very clear about what I was hoping for; it's pretty simple actually, just awkward to explain.
I'm thinking that \marginpar with no optional argument creates an <ltx:note role="margin" class="ltx_marginpar".... But if both arguments are given, it should create two ltx:note, one with class="ltx_marginpar_left", using the optional argument, the other with class="ltx_marginpar_right" using the required argument.

Then, if you want notes in the right margin, you'd use CSS to add some space on the right, and make both ltx_marginpar and ltx_marginpar_right be visible, positioned to the right, and make ltx_marginpar_left have display:none. For left margins, you'd add the space on the left and position ltx_marginpar and ltx_marginpar_left notes to the left and make ltx_marginpar_right invisible. And there's sufficient data there to do fancy things if you want to play more and add javascript.

The \reversemarginpar would basically be ignored as it (probably) doesn't correspond to anything you'd want to do in html.

Does that make more sense?

@dginev
Copy link
Collaborator

dginev commented Jul 27, 2023

I really like that proposal actually!

@teepeemm
Copy link
Contributor Author

I think that ends up as one of

DefConstructor('\marginpar[]{}',
  "?#1(?#2("
      ."<ltx:note role='margin' class='ltx_marginpar_left'><ltx:inline-para>#1</ltx:inline-para></ltx:note>"
      ."<ltx:note role='margin' class='ltx_marginpar_right'><ltx:inline-para>#2</ltx:inline-para></ltx:note>)"
    ."(<ltx:note role='margin' class='ltx_marginpar'><ltx:inline-para>#1</ltx:inline-para></ltx:note>))"
  ."(<ltx:note role='margin' class='ltx_marginpar'><ltx:inline-para>#2</ltx:inline-para></ltx:note>)");
# or
DefConstructor('\marginpar[]{}', sub {
    my ($document, $left, $right) = @_;
    if ($left) {
      $document->openElement('ltx:note', role => 'margin', 'class' => 'ltx_marginpar' . ($right ? '_left' : ''));
      $document->insertElement('ltx:inline-para', $left);
      $document->closeElement('ltx:note'); }
    if ($right) {
      $document->openElement('ltx:note', role => 'margin', 'class' => 'ltx_marginpar' . ($left ? '_right' : ''));
      $document->insertElement('ltx:inline-para', $right);
      $document->closeElement('ltx:note'); } });

Do you have a preference on which one? I think using the sub is more readable, but it is 4 lines longer.

Somewhat related, but I'm having some sort of trouble compiling. No matter what I do to LaTeX.pool.ltxml, it is somehow using the file from the prior commit, so I'm not able to test this out. Do you know if this is a git thing, or a LaTeXML thing, or a Perl thing, or some strange thing that you can't quite tell from my vague description? I've tried running make, as well as git add the file to be committed. (Could it be relevant that git status keeps saying that the file has changes to be committed as well as changes not staged for commit?)

@brucemiller
Copy link
Owner

I tend to prefer the xml pattern forms, when they're not too complex; plus you probably can assume that there will be an arg 2, so you should be able to use:

DefConstructor('\marginpar[]{}',
  "?#1("
      ."<ltx:note role='margin' class='ltx_marginpar_left'><ltx:inline-para>#1</ltx:inline-para></ltx:note>"
      ."<ltx:note role='margin' class='ltx_marginpar_right'><ltx:inline-para>#2</ltx:inline-para></ltx:note>)"
    ."(<ltx:note role='margin' class='ltx_marginpar'><ltx:inline-para>#2</ltx:inline-para></ltx:note>)");

make and running latexml normally shouldn't be affected by git; just what's in your working directory; Or the local directory where you're testing from. Maybe there's a stray copy of LaTeXML.pool.ltxml sitting somewhere that LaTeXML is picking up instead of the one from git directory? The log of your test should show where things were fetched from, for example

...
(Loading /home/bruce/LaTeXML/blib/lib/LaTeXML/Package/LaTeX.pool.ltxml...
...

@brucemiller
Copy link
Owner

Oh, if a file has staged changes and local changes, that can make things like git diff confusing, but still it will be version in the working directory that latexml uses when it runs.

@teepeemm
Copy link
Contributor Author

Ah. I'd copied LaTeX.pool.ltxml into my working directory so that my git fumblings wouldn't overwrite what I was trying to do.

Deyan did have a possible example where there would only be the optional argument: if the author was sure that the marginpar was going to be on an even page. So I think we do need to watch out for that possibility.

@brucemiller
Copy link
Owner

brucemiller commented Jul 28, 2023 via email

@brucemiller
Copy link
Owner

Moreover, whatever the author had in mind, the text provided in the optional arg should not be used on the right margin and should still have ltx_marginpar_left, not ltx_marginpar.

@teepeemm
Copy link
Contributor Author

I believe this incorporates everything we've discussed. If there's #1, it gets .ltx_marginpar_left and #2 (if present) gets .ltx_marginpar_right. Otherwise, there's only #2, and we just use .ltx_marginpar (this assumes no one will do a completely empty \marginpar{}). By default, the CSS hides the _left version.

Copy link
Collaborator

@dginev dginev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool! I haven't tested it on a real-world example, but the code looks solid.

@brucemiller
Copy link
Owner

Nice! I think this will work well by default, as well as with the optional LaTeXML-marginpar.css, and still provide users the flexibility to do left marginpars or other fancy stuff if they wish. Thanks!!

@brucemiller brucemiller merged commit a4804e2 into brucemiller:master Aug 4, 2023
13 checks passed
@teepeemm teepeemm deleted the marginpar branch August 5, 2023 22:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants