#232 no-click adding doesn't sync?

[Lee Worden]

JD reports:

I experienced an interesting glitch with this new feature. I entered a .tex file, and it was automatically added, but the associated default target was not made successfully. The make log reported "no rule to make". IOW, it apparently tried to make the associated default target before it sync-ed the file. Previewing again solved it, but this will confuse some people.

[Jonathan Dushoff]

Relatedly (maybe not interesting) just experienced this problem again where the project-file tag was above the new source file (as why shouldn't it be?).

[Lee Worden]

Right, this occurs during preview, because during preview WW syncs all source files the first time there's a need to make, and not again; but it adds the new source files after the sync, so they get omitted.

Why does it do syncing differently when previewing again? It's because the preview page's text isn't stored on the server anywhere. Thus when a project file is requested, say an image made by GetProjectFile, you can't sync source files that are on that page because you don't have them. So in a preview session, we only sync files that aren't on the page that's being edited. The files on the page get synced in a special operation when the page is previewed.

Looks like I'll need to make that work with the add operation.

[Lee Worden]

I could have it keep track of which source files it's synced, and try to sync every time it makes something. This would cause no overhead for multiple project files on the page in general, because everything would have been synced the first time; but it would catch the stragglers. But this wouldn't work when a project file occurs before the source file. Maybe that's okay?

[Lee Worden]

I have to look at the code. All the making is done in a separate pass after the page is parsed. Maybe I can get the source files added before any of the making?

[Lee Worden]

This is sort of a legacy-bug-to-be, because the switch to jquery stuff is likely to make it go away. In the meantime, JD and I agree that it should include a warning that the page should be reloaded.

[Lee Worden]

I should write more clearly. The situation is this: WW handles source-file and project-file tags as it encounters them in the page, from beginning to end. Also, when it adds a new source-file to the project, it doesn't sync its contents into the working directory on the spot. During preview, the source files only get synced at the beginning of the page. So the source files that got added along the way aren't there for make rules that need them. If WW did sync them on the spot, it would probably help with make operations that happen further down the page from the source files they need. But we want project-file tags earlier on the page to work as well.

[Lee Worden]

Let me look in the code, to find out for sure what happens when.

1. While MediaWiki's parser is reading the wikitext, it calls WWInterface::source_file_hook() and WWInterface::project_file_hook(). These functions record the tag attributes and place a short token in the wiki text, to be dealt with later.

2. Once the parser is done, WWInterface::render_after_parsing() transforms the page's text by finding all the tokens and replacing them by the actual file contents, using a render_token() call for each, in order. This is where all the processing is done, including adding source files to projects as well as extracting source file contents from their pages and syncing them to the directory, and running make to do the computations.

The click to add or add-without-clicking happens in render_token's first step, where it figures out which project to associate the file with. The second step is to call display_project_file() to make and/or display the file contents.

So I guess I could split render_token() into two separate functions, and do all the project associating followed by all the file displaying.

The special early sync for preview sessions happens in WWPreview::RenderProjectFile_hook(), which, as the name sort of suggests, is called by render_token(). So it shouldn't be too hard to make sure that happens after all the add-without-clicking but before the making.

Or it might be workable to do the project associating and adding in the parser hooks, and just do the making and displaying afterward, I don't know whether there's a downside to that.

[Lee Worden]

the parser hook functions appear to do the token setup using only project names, without loading the project data, but it's not true. They use project_name_for_tag(), which loads in the ProjectDescription object it needs before asking its name. This could be fixed to remove the overhead of loading the project, but on the other hand since it's there I could put the add-file stuff in here without LOSING any efficiency. Well, roughly, since it will also have to load the parent pages' projects sometimes.

But - all this is likely to get rewritten to do asynchronous making and displaying. So I should just do whatever's most straightforward right now.

render_token currently does:

• Find the tag data for the token in WWInterface::$uniq_tokens
• Process it if it's a <toggle-make-enabled>, otherwise it's <source-file> or <project-file> and needs more steps:
• Check the filename and project name, get ProjectDescription object for project
• WW-RenderProjectFile hook (which handles syncing all files during preview)
• If a source file, check whether the project expects it to be here. If not, either correct the project automatically or ask the user what to do, depending on whether there's any potential controversy.
• If a project file, check whether the project needs to be updated regarding its location or archived/non-archived state. If so, either do it automatically or ask the user for instructions.
• call display_project_file to make and display the file contents.

[Lee Worden]

The first pass should be

• Check filenames and project names, load the project objects
• Check and update the project data

and the second pass

• Process <toggle-make-enabled> directives interleaved with the make operations
• WW-RenderProjectFile (needs to happen after all the project updating)
• display_project_file (and error reporting).

It might be that each pass produces some error reporting, and we'll end up with an error output box saying "Which project to add file to?" right up against another one saying "Make failed" or whatever, when it'd be better to have them in a single box, but not a big deal. And the error reporting will be done differently when we go asynchronous.

[Lee Worden]

Instead of calling render_token() in a loop, I'm retiring render_token() and putting its content into render_after_parsing() inline, refactored into two loops.

[Lee Worden]

seems to work. will test more before inflicting on others.

[Lee Worden]

going live on lalashan. eyes out for problems.

[Lee Worden]

This is letting some UNIQ_* tokens through on lalashan, but unfortunately the only examples I got were from a complex, multi-page project, so it would be hard to copy it to another wiki for testing. Since I backed out the change on lalashan, I'm going to have to look for another way to test it. Might be worth export/importing the big project (and installing the site makefiles on the test wiki, and trying to get a compatible R installation running... or testing on a wiki on lalashan, of course).

Anyway, the changes also change the default make behavior somehow... <project-file filename=X.pdf/> apparently uses make=no instead of make=yes. It's not obvious to me why, because the line from render_token that handles that is installed correctly in the final loop in render_after_parsing in the revised code.

[Lee Worden]

Well, I guess I should track down the make=false thing, since I can do that on the test wiki, and see where else it might lead...

[Lee Worden]

OK I'm currently looking at a preview page on lalashan (at r981, before these changes) and one on my localhost at r982, after these changes. I've confirmed that before the changes, a <project-file filename=X.pdf/> tag tries to make the pdf before displaying its link, and after the changes it doesn't.

So far the relevant code looks the same in both cases, so I'll have to look closer.

[Lee Worden]

The old version:

• project_file_hook() with no make= records its args in $uniq_tokens as is, i.e. without any 'make' entry.
• render_after_parsing() finds the UNIQ_ token, and calls render_token() directly without messing with the args.

• render_token() recovers $args from $uniq_tokens, passes it to the RenderProjectFile hook, and then does

  if (isset($args['make']))
  #$args['make'] = 0;
  $args['make'] = wwfArgumentIsYes($args['make']) ? 1 : 0;

• and passes $args into display_project_file().

The new version:

• project_file_hook() is untouched: it records $args without a 'make' entry.
• I don't know whether to say render_after_parsing() does two passes or three now. Let's go with three. So the first one is finding all the UNIQ_ tokens and recording where they are. In the second pass, where it does the clickToAdd stuff, it retrieves $args but doesn't do anything with $args['make'].

• The third pass retrieves $args again, passes it into the RenderProjectFile hook, then does

  if (isset($args['make']))
  $args['make'] = wwfArgumentIsYes($args['make']) ? 1 : 0;

• and passes to display_project_file().

In both cases, both the background and preview code implement RenderProjectFiles hooks, but both are unchanged. The background one handles the make=background case, and the preview one adds a sync operation the first time it's called, if needed.

So I think these differences in the code aren't the reason for the change in behavior. It must be something more elusive, like it's using the $args for a different project-file tag or something. Need to comb through some log output...

[Lee Worden]

Also, in both cases, display_project_file() is unchanged, and it uses make=true or make=false if it's given a value, and if $args['make'] is unset then it deduces which to do:

if (!isset($args['make'])) // or $args['make'] == 0)
{ if ($source)
  { if ($target !== $args['filename'])
      $args['make'] = true;
    else
      $args['make'] = false;
  }
  # don't do this - better to be consistent regardless of display mode
  #else if ( $display_mode == 'link' or $display_mode == 'none')
  #  $args['make'] = false;
  else
    $args['make'] = true;
  #wfDebug("set make to {$args['make']}.\n");
}

... ok, got it. It's because render_after_parsing() didn't set $source in the last pass. So it was using whatever value $source had for the last tag on the page, left over from the previous pass.

[Lee Worden]

*#$%(*# just lost a long list I typed in here.

But okay, the usefulness was in typing it, not saving it (I hope).

After lifting bits of wikitext from the buggy page on lalashan and trying various parts of it, I think the stray UNIQ_ might be connected to __DISABLE_MAKE__.

Yes, that part of the code isn't updating $newtext properly. (Sorry, you'd know what $newtext is if I hadn't lost all that typing...)

Sure enough, I had a stray UNIQ_ on the page and once I fixed that it went away. Maybe it's safe to push onto lalashan again?

I think I'll do it tomorrow, so I'll be around in case it goes awry again.

[Lee Worden]

OK, rolling it out again, with fixes.

[Lee Worden]

More bugs reported. In this case, UNIQ_ tags are visible where $$...$$ constructs are entered. $$ constructs are working for me in simple tests, but not looking so good in the presence of a $$ construct in a header, for instance

== Free-drug environment ($$\phi=0.0$$) ==

There is some special code for that case, to avoid garbled html anchor names, so it's not a great surprise. Backed out code again, looking at this case.

[Lee Worden]

Well, for one thing, in this case the UNIQ tokens aren't unique, because the one I put there ends up in both the <h2> text and the anchor name, i.e. the HTML looks like

<a name="...UNIQ-WW-n-QINU..."></a><h2>...UNIQ-WW_n_QINU...</h2>.

So I've got to be careful how it gets handled in $uniq_tokens. That code's already written and tested, but I think I've introduced some kind of failure into it during the rewrite. (I.e. it's supposed to recognize the text that's in the anchor name, and not process it, and I think the recognition isn't working for some reason.)

[Lee Worden]

To be precise, in my test wiki (MW 1.19) it shows up in three places:

<h2><mw:editsection page="Sandbox" section="1">Free-drug environment 
(UNIQ-WW-273954845-QINU)</mw:editsection> <span class="mw-headline"
id="Free-drug_environment_.28UNIQ-WW-273954845-QINU.29"> 
Free-drug environment (UNIQ-WW-273954845-QINU) </span></h2>

and it looks like my code is correctly ignoring the one that's in an id attribute, but not the other two.

I believe the mw:editsection tag is used internally, and replaced by the [edit] link before the final HTML. I'm not positive whether it's better to expand out the token there or leave it alone (i.e. will it be used as text or as a tag attribute).

Regardless of the details, I think it's a bad idea to store stuff in $uniq_tokens that isn't unique, i.e. the start and ending location of the token on the page, if it might show up in more than one place. I should put those values in $tokens instead, so they can be multiple.

[Lee Worden]

Well, that was just a guess, but it does seem to fix the problem right up. I'll roll it back onto lalashan and see how it works there.

[Lee Worden]

Belay that. Not fixed enough yet. There's some garbling in the mw:editsection, I think. Have to try not expanding that part.

... yes, that gets the header text and edit link right. Unfortunately, the edit link has a popup text (i.e. a "tooltip") that says "Edit section: Free-drug environment (UNIQ-WW-273954845-QINU)". This will probably put some people off. But expanding the UNIQ- into its mathml output causes the whole edit section to break, so the only better solution I can think of would be to put the unprocessed LaTeX source code in there, which is really ad-hoc and would require a whole special case with code to bypass the default display= and what not. So I'll leave it for now.

[Lee Worden]

I could change "UNIQ-WW-*-QINU" into literally "..." in the section anchors and tooltips =. But this would make the anchors non-unique, i.e. if you have sections "$$\phi=0.1$$", "$$\phi=0.2$$", etc., as the page where this bug appeared does have, the links in the TOC would all go to the first of these sections.

I'll open a bug about putting the latex source in the anchors and tooltips (if there isn't one already), and leave it for another day.