-
Notifications
You must be signed in to change notification settings - Fork 35
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
Support for odt export in datatable and datalist #169
base: master
Are you sure you want to change the base?
Conversation
Nice idea, looks fine for me. |
Thanks. I tried to fix a few more options in _formatData, but this way I'm definitely losing the title attribute when rendering type 'tag'. |
@andyboeh this currently breaks tests. Can you please look into it? |
I just noticed that it doesn't only break tests, it even changes the behaviour of the _formatData function. The reason is that $R->internallink has an option returnonly, which is not present for $R->emaillink and $R->externallink. Now _formatData sometimes renders to $R->doc directly instead of returning the rendered code. However, IMHO using the renderer's emaillink and externallink function is necessary to support different renderers. Would it be possible to add a returnonly option to emaillink and externallink? |
sure. open a PR
|
This now needs dokuwiki/dokuwiki/pull/1239 to be merged before the tests succeed. |
Does missing of the update provided by #1239 breaks existing, not yet updated, wikis? |
@@ -220,7 +220,7 @@ public function ensureAbsoluteId($id) { | |||
* @param Doku_Renderer_xhtml $R |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please update the type in the PHPDocs.
Can you add also PHPDocs for the functions where your add new arguments to the function?
(and just a suggestion: some IDEs can generate the PHPDocs for you (and perform many other inspections as well). For example you could have a look at https://www.dokuwiki.org/devel:intellij_idea )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, I'll update that. You mean the functions in #1239? I tried to comment every changed function signature, did I miss some?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean in the files helper.php
, syntax/list.php
etc.
For example: Doku_Renderer_xhtml
at line 220, can now be updated to Doku_Renderer
Further the before_val, before_item,after_...(), etc doesn't have PHPDocs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Note: this are inline comments, maybe in the Github view two discussions are mixed)
Yes, I think it's going to break older wikis - I'll test that. |
Updated the documentation and added a check for the DokuWiki version. It runs either the old or the new formatting functionality, based on the Wiki version. |
Ich kehre zur�ck am 10.08.2015. Ich werde Ihre Nachricht nach meiner R�ckkehr beantworten. Herzlichen Dank I will answer your message after returning. Hinweis: Dies ist eine automatische Antwort auf Ihre Nachricht "Re: Diese ist die einzige Benachrichtigung, die Sie empfangen werden, w�hrend |
Regarding the failing test: I honestly believe that the output of the new _formatData function is valid (according to the HTML spec), although the output changes slightly (not visibly!). |
Ich kehre zur�ck am 10.08.2015. Ich werde Ihre Nachricht nach meiner R�ckkehr beantworten. Herzlichen Dank I will answer your message after returning. Hinweis: Dies ist eine automatische Antwort auf Ihre Nachricht "Re: Diese ist die einzige Benachrichtigung, die Sie empfangen werden, w�hrend |
} else { | ||
$title = hsc($title); | ||
} | ||
$outs[] = $R->emaillink($id, $title, true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The _formatDataOld() has some rawurlencoding applied, which is applied to the content of the title
attribute as well. For the title attribute this encoding is not required.
My proposal is to improve the unittest and to modify the behavior in the _formatDataOld() such that it matches the new behavior.
The old code reuses the encoded $id:
if($conf['mailguard'] == 'visible') {
$id = rawurlencode($id);
}
$outs[] = '<a href="mailto:' . $id . '" class="mail" title="' . $id . '">' . $title . '</a>';
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ich kehre zur�ck am 10.08.2015.
Ich werde Ihre Nachricht nach meiner R�ckkehr beantworten. Herzlichen Dank
f�r ihr Verst�ndnis.
I will answer your message after returning.
Hinweis: Dies ist eine automatische Antwort auf Ihre Nachricht "Re:
[dokuwiki-plugin-data] Support for odt export in datatable and datalist
(#169)" gesendet am 30.07.2015 22:50:48.
Diese ist die einzige Benachrichtigung, die Sie empfangen werden, w�hrend
diese Person abwesend ist.
This should fix the remaining tests. I had to change the output of the tag generation in _formatDataOld as well as URL extern generation. Before tests succeed, PR dokuwiki/dokuwiki/pull/1275 needs to be merged. Afterwards, the date checks need to be updated (note to myself). |
Date checks should be up to date now and tests pass - any further comments? |
We needed to export a big datatable to ODT and were quite surprised to find out that the data plugin currently doesn't support rendering to odt.
This PR adds basic support for datatable and datalist (untested).
Before I continue fixing the remaining options in the helper's _formatData(), could somebody please comment on this?