01098: Preview page text variables and pagelist templates

Summary: Preview page text variables and pagelist templates
Created: 2009-06-04 23:02
Status: Closed - fixed for 2.2.9
Category: CoreCandidate
From: RandyB
Priority: 55555 442
Version: All

Description: Preview purports (intuitively if not explicitly) to show the user how the page will look if saved. Page text variables however are currently evaluated within curly braces by using the values on the old page. If those values are being changed, then the preview is inaccurate and confusing, making preview unusable in some applications.

Whether or not you consider this a bug or just unintuitive program behavior, preview should be as accurate as practical. In this case, it's an easy fix: Peter Bowers wrote a patch that could be included in the core, to make this problem go away. RandyB has tested it so far with no problems.

Hi - I definitely vote for either fixing this, or allowing admins a switch to change the behavior. My trouble involves pagelists, PTVs and [[!Category]] markup. see http://www.pmwiki.org/wiki/Test/PTVandCategory for an example. Basically, when saving something like (:Keyword: [[!myKeyword]]:){$:Keyword} to a page, one actually has to save the page twice in order for a pagelist with link=Category.Keyword to accurately list that page. This is confusing, and was hard (for a novice like myself) to track down (I inadvertently blamed Fox and FoxEdit for the first few days of my problem). Plus, if a user were to type in the category+PTV markup and only save once, the pagelist might never be updated to be correct.

I wonder - is there a good reason for leaving it the way it is? For now, on my site, it doesn't seem to make much sense, and it seems more intuitive to allow category markup written within a PTV to get logged on the first save, just as it would if one had typed it directly on the page.
my 2cents. thanks for considering!
overtones99 June 19, 2009, at 02:58 PM

I fully support changing this. To mepreview is providing a view of the page as it would look if it was saved.
It is counter-intuitive to have parts of the page (such as PTVs or pagelist templates) sourced from the last saved version and other parts sourced from the screen.

Simon June 21, 2009, at 03:52 PM

Hi. I just tried out the patch and it did NOT work for my issue. on my system (just upgraded to 2.2.2) it still takes two saves to get links in PTV's to show up in pagelist output (or in pageindex targets), even with pmwiki.php patched up. someone else might want to try it out as well to confirm. Thanks.
overtones99 June 21, 2009, at 07:34 PM

I went ahead and uploaded the patch if it'll be helpful. Apparently the *.patch extension is not allowed in the upload section so I've named it Attach:preview-fix.txt instead. Just apply it with the "patch" utility.

Caveat: note that making changes to pmwiki.php is not recommended. In particular if you ever upgrade and this patch has not been applied to core then you will need to remember to re-apply the patch on every upgrade. --Peter Bowers

Updated the patch to deal with the problem encountered by Adam with hidden PTVs containing categories but the link= not being updated in the .pageindex until the 2nd save. -Peter Bowers June 26, 2009, at 06:43 AM

i can confirm that it appears to work on my end without any particular side-effects to date. thanks Peter! would love for such a thing to be part of the core to prevent endless patching with future updates. PTV, pageindex, and pagelist link= activity now seem to do what i intuntively expect them to do.
thanks! overtones99 June 26, 2009, at 05:38 PM

I have just created a pre-release dealing with these issues (previews for PTVs, PageList templates, self-includes and trails) which is in SVN revision 2350 (diff). I'll appreciate any testers and reviewers. Note that I used a different approach than the one suggested by Peter Bowers. Also please don't build structure over this version as things may change. Thanks in advance. --Petko July 05, 2009, at 06:45 AM

With SVN revision 2350 my preview shows the PTVs currently in the page rather than the values as they will be upon saving. If it's supposed to be a replacement for Peter's patch, it's not working for me. --RandyB

The thing is enabled on pmwiki.org, and it works for me. If I change one of the PTVs at the top of the page, and press Preview, I see here below the modified value. Testing: "RandyB" "Closed - fixed for 2.2.9" "55555 442". --Petko July 05, 2009, at 12:52 PM

My testing environment is down indefinitely so I'm kind of dead in the water in terms of testing/developing. However, I'd like to make a "plug" for the earlier patch I offered. Giving the capability of caching a page using page-text from a variable rather than requiring reading from a page expanded flexibility and capabilities in addition to fixing the problem -- it's something recipe authors can make use of in other contexts. The new svn version deals specifically with the one issue (I could be wrong, but I don't think it will fix Adam's issue) but doesn't add any flexibility. In the end it may be 6.0001 of one and half a dozen of the other, but I think the caching solution does have some advantages without losing anything...

Just my $0.02. -Peter Bowers July 09, 2009, at 08:27 PM

PageTextVariables are already cached in the $PCache array. In Adam's test page (click here), when the page is saved, the category listing is correctly updated. --Petko July 10, 2009, at 03:00 AM

About "caching a page using page-text from a variable rather than requiring reading from a page... something recipe authors can make use of in other contexts", I wonder if it is likely to be needed a lot? If not, such a function could be added in a recipe which requires it. --Petko July 12, 2009, at 09:55 PM

I wanted Pm to review this change before releasing it, but he is extremely busy at the moment, and I have to cut a new release in the shortest delays. It is in the latest svn and will likely get in 2.2.3. This new feature should be considered experimental and may change/disappear. It will be disabled by default; place in config.php $EnablePreviewParse=1; to enable it for testing purposes only. Comments welcome as always. --Petko July 12, 2009, at 08:40 AM

Pm suggested a different implementation, I started but I didn't have the time to finish it. So I removed it for the 2.2.3 release and I'll look at it in the next few days. It is currently no more enabled on pmwiki.org. --Petko July 15, 2009, at 08:38 PM