152 lines
8.1 KiB
Markdown
152 lines
8.1 KiB
Markdown
Very nice! There are some rough spots yes, but this looks exactly as I'd
|
|
hoped it would, and seems close to being ready for merging.
|
|
|
|
A few observations, in approximate order of priority:
|
|
|
|
* What's the copyright and license of showdown? Please include that from
|
|
the original zip file.
|
|
* Done. Check licences folder
|
|
* What happens if there are concurrent edits? The CGI.pm modification to
|
|
save an edited wikiwyg part doesn't seem to check if the source file has
|
|
changed in the meantime, so if the part has moved around, it might
|
|
replace the wrong part on saving. I've not tested this.
|
|
* When you click the edit button, the exact same protocol is used for saving.
|
|
However when you double click to edit, this still is possibly an issue.
|
|
* The stuff you have in destdir now really belongs in basewiki so it's
|
|
copied over to any destdir.
|
|
* Done.
|
|
* Personally, I'm not sure if I need double-click to edit a section in my
|
|
wiki, but I'd love it if the edit form in the cgi could use wikiwyg. Seems
|
|
like both of these could be independent options. Doable, I'm sure?
|
|
* Done.
|
|
* It would be good to move as much as possible of the inlined javascript in
|
|
wikiwyg.tmpl out to a separate .js file to save space in the rendered
|
|
pages.
|
|
* Done.
|
|
* Both this plugin and the [[Gallery]] are turning out
|
|
to need to add a bunch of pages to the basewiki. I wonder what would be a
|
|
good way to do this, without bloating the basewiki when the plugins arn't
|
|
used. Perhaps the underlaydir concept needs to be expanded so it's a set
|
|
of directories, which plugins can add to. Perhaps you should work with
|
|
arpitjain on this so both plugins can benefit. (The smiley plugin would
|
|
also benefit from this..)
|
|
* Done. All plugin files are now stored in a tarball. IkiWiki checks for
|
|
<plugin name>.tar.gz in the basedir and if the plugin is being used, then
|
|
it extracts the files to destdir. Currently IkiWiki does not render these
|
|
files though (my plugin doesn't need them to be rendered). However it wouldn't
|
|
be too hard to modify it to render them.
|
|
* Is there any way of only loading enough of wikiwyg by default to catch
|
|
the section double-clicks, and have it load the rest on the fly? I'm
|
|
thinking about initial page load time when visiting a wikiwyg-using wiki
|
|
for the first time. I count 230k or so of data that a browser downloads
|
|
in that case..
|
|
* Done-ish. I fixed it so that all of the javascript files(except for the main two)
|
|
are loaded after the content is loaded. It is possible to make is so that
|
|
the files are only loaded when you double click, however that is *a lot*
|
|
more work, plus it will slow the load time for wikiwyg. But if you would
|
|
prefer that the files only load after double clicking, I can do that. Also,
|
|
I'm working on reducing the file sizes via [Javascript Compression][]. Theoretically,
|
|
I can get the size down to about 70kb, I'm working out the kinks now.
|
|
|
|
--[[Joey]]
|
|
|
|
Oh, by the way, let me know if I forgot to tarball anything. --[[TaylorKillian]]
|
|
|
|
[Javascript Compression]: http://javascriptcompressor.com/
|
|
|
|
---
|
|
|
|
Some more comments, on version 1.6. You seem to be making nice progress.
|
|
|
|
changes.diff:
|
|
|
|
* I don't really like the tarball approach. Doesn't feel like the right
|
|
approach somehow. A list of underlay directories feels to me like a
|
|
better approach. One reason is that it's more general than a tarball tied
|
|
to a given plugin. A list of underlay directories could also be used to
|
|
prefer a translated underlay, and use the english version of untranslated
|
|
pages, for example.
|
|
* I don't quite get what you want to do with the underlay directory, it sounds like
|
|
you have something pretty specific in mind. I can talk to you about that more
|
|
on IRC later(assuming my internet is working right).
|
|
* Basically the idea is to change `$config{underlaydir}` to an array..
|
|
Ok, take a look at the new `add_underlay()` function. You can now just
|
|
`add_underlay("wikiwyg")` and it'll look in
|
|
/usr/share/ikiwiki/wikiwyg/ for the files.
|
|
* When is the WIKIWYG variable in misc.tmpl used?
|
|
* The WIKIWYG variable in misc.tmpl is used for the edit page. I believe that is what
|
|
you wanted me to do (Check [Revision 3840][]).
|
|
* Ah, right.
|
|
* Could you move the code that handles saving a page of the page into the
|
|
plugin? I just added an editcontent hook, which should allow you to do
|
|
that.
|
|
* Alright, np.
|
|
* Your patch exports run_hooks, but I don't see the plugin using that.
|
|
* Yeah, that was from an earlier revision of my plugin, I just forgot to remove that.
|
|
* I don't know about exporting pagetitle. So far, only the inline plugin
|
|
needs to use that function, I generally only export things after it's
|
|
clear a lot of plugins will need them.
|
|
* Just looked through the inline plugin. So if I want to use pagetitle in my code,
|
|
I have to use the IkiWiki package instead of IkiWiki::Plugin::Wikiwyg? Or would a
|
|
better approach be to just copy that function into the Wikiwyg plugin?
|
|
* You can just call `IkiWiki::pagetitle()`.
|
|
|
|
wikiwyg.tar.gz
|
|
|
|
* Would it be possible to provide a diff between wikiwyg upstream and any
|
|
modifications you made to its files? I'm not sure which version you used,
|
|
so I'm seeing changes in diffing that I'm unsure if you made..
|
|
* <http://ikiwiki.xbaud.com/JavaScript_Diffs.tar.gz>, also emailed them to you
|
|
in case my internet goes down.
|
|
* Could you redo that with diff -u plz?
|
|
* Link is updated
|
|
* If the files aren't modified, would it be better for users to get them
|
|
from the wikiwgy upstream, instead of including them in the plugin? (If so,
|
|
they'd go in their own Debian package..)
|
|
* The files *are* modified, but I doubt it will make a difference. There have
|
|
been no updates to Wikiwyg since 5/30/07 so I'm pretty sure it's unmaintained
|
|
now. Showdown is the same case, they haven't changed anything since SoC began.
|
|
I could separate them diff's though if you feel it is worth it.
|
|
* Well, from a packaging perspective, the question is whether some
|
|
other package might want to use the wikiwyg/showdown javascript
|
|
files. And whether your mods might break that. If the answers to
|
|
these questions are yes and no, then it would make sense to package
|
|
them as standalone packages rather than embedding them in ikiwiki.
|
|
|
|
misc:
|
|
|
|
* What are your thoughts on handling plugins? Just make preview do a
|
|
server-side callback?
|
|
* That is an option, however I was trying to avoid that due to bandwidth, cpu time
|
|
concerns (Two reasons I really like IkiWiki). I was planning on just manually
|
|
implementing some of the easier ones (such as img), however I'm still trying to
|
|
think of a way for the more complex ones.
|
|
* It just seems like it would never be able to support everything,
|
|
and would mean reimplementing stuff in javscript and would constantly
|
|
need to be kept up to date. Ikiwiki's preview is actually pretty
|
|
fast, the only real overhead being the cgi call.
|
|
* How do I configure it to only support whole-page editing with wikiwyg and
|
|
not insert the javascript into html pages?
|
|
* There currently is no option to do that, however it is a 2 line change that I'll work
|
|
on after I finish typing this.
|
|
* When editing a whole page with wikiwyg, I think it would be good to keep
|
|
the save, preview, cancel buttons at the bottom like they are in a
|
|
regular page edit. Also the comments box. Kind of a least suprise thing, so that enabling
|
|
wikiwyg for whole-page editing basically just changes how the edit box
|
|
behaves and keeps the rest of the behavior the same. And I think the preview
|
|
button should show a preview rendered server-side, like with a regular edit,
|
|
since such a preview is able to support all plugins.
|
|
* That's probably a good idea ;)
|
|
|
|
Everything else looks fine and ready for merging. If, that is, you think
|
|
I should include the plugin with all of its java code in ikiwiki. Thoughts?
|
|
|
|
--[[Joey]]
|
|
|
|
I'll start working on the changes... Let me know if you find anything else
|
|
that needs to be changed. I'd be honored to have my code merged with IkiWiki :)
|
|
|
|
--[[TaylorKillian]]
|
|
|
|
[Revision 3840]: http://ikiwiki.info/cgi-bin/viewvc.cgi?view=rev&root=ikiwiki&revision=3840
|