| blob | 304b3962095361a549a68f8c3ef199f69490146e |
1 Very nice! There are some rough spots yes, but this looks exactly as I'd
2 hoped it would, and seems close to being ready for merging.
4 A few observations, in approximate order of priority:
6 * What's the copyright and license of showdown? Please include that from
7 the original zip file.
8 * Done. Check licences folder
9 * What happens if there are concurrent edits? The CGI.pm modification to
10 save an edited wikiwyg part doesn't seem to check if the source file has
11 changed in the meantime, so if the part has moved around, it might
12 replace the wrong part on saving. I've not tested this.
13 * When you click the edit button, the exact same protocol is used for saving.
14 However when you double click to edit, this still is possibly an issue.
15 * The stuff you have in destdir now really belongs in basewiki so it's
16 copied over to any destdir.
17 * Done.
18 * Personally, I'm not sure if I need double-click to edit a section in my
19 wiki, but I'd love it if the edit form in the cgi could use wikiwyg. Seems
20 like both of these could be independent options. Doable, I'm sure?
21 * Done.
22 * It would be good to move as much as possible of the inlined javascript in
23 wikiwyg.tmpl out to a separate .js file to save space in the rendered
24 pages.
25 * Done.
26 * Both this plugin and the [[Gallery]] are turning out
27 to need to add a bunch of pages to the basewiki. I wonder what would be a
28 good way to do this, without bloating the basewiki when the plugins arn't
29 used. Perhaps the underlaydir concept needs to be expanded so it's a set
30 of directories, which plugins can add to. Perhaps you should work with
31 arpitjain on this so both plugins can benefit. (The smiley plugin would
32 also benefit from this..)
33 * Done. All plugin files are now stored in a tarball. IkiWiki checks for
34 <plugin name>.tar.gz in the basedir and if the plugin is being used, then
35 it extracts the files to destdir. Currently IkiWiki does not render these
36 files though (my plugin doesn't need them to be rendered). However it wouldn't
37 be too hard to modify it to render them.
38 * Is there any way of only loading enough of wikiwyg by default to catch
39 the section double-clicks, and have it load the rest on the fly? I'm
40 thinking about initial page load time when visiting a wikiwyg-using wiki
41 for the first time. I count 230k or so of data that a browser downloads
42 in that case..
43 * Done-ish. I fixed it so that all of the javascript files(except for the main two)
44 are loaded after the content is loaded. It is possible to make is so that
45 the files are only loaded when you double click, however that is *a lot*
46 more work, plus it will slow the load time for wikiwyg. But if you would
47 prefer that the files only load after double clicking, I can do that. Also,
48 I'm working on reducing the file sizes via [Javascript Compression][]. Theoretically,
49 I can get the size down to about 70kb, I'm working out the kinks now.
51 --[[Joey]]
53 Oh, by the way, let me know if I forgot to tarball anything. --[[TaylorKillian]]
55 [Javascript Compression]: http://javascriptcompressor.com/
57 ---
59 Some more comments, on version 1.6. You seem to be making nice progress.
61 changes.diff:
63 * I don't really like the tarball approach. Doesn't feel like the right
64 approach somehow. A list of underlay directories feels to me like a
65 better approach. One reason is that it's more general than a tarball tied
66 to a given plugin. A list of underlay directories could also be used to
67 prefer a translated underlay, and use the english version of untranslated
68 pages, for example.
69 * I don't quite get what you want to do with the underlay directory, it sounds like
70 you have something pretty specific in mind. I can talk to you about that more
71 on IRC later(assuming my internet is working right).
72 * Basically the idea is to change `$config{underlaydir}` to an array..
73 Ok, take a look at the new `add_underlay()` function. You can now just
74 `add_underlay("wikiwyg")` and it'll look in
75 /usr/share/ikiwiki/wikiwyg/ for the files.
76 * When is the WIKIWYG variable in misc.tmpl used?
77 * The WIKIWYG variable in misc.tmpl is used for the edit page. I believe that is what
78 you wanted me to do (Check Revision 3840).
79 * Ah, right.
80 * Could you move the code that handles saving a page of the page into the
81 plugin? I just added an editcontent hook, which should allow you to do
82 that.
83 * Alright, np.
84 * Your patch exports run_hooks, but I don't see the plugin using that.
85 * Yeah, that was from an earlier revision of my plugin, I just forgot to remove that.
86 * I don't know about exporting pagetitle. So far, only the inline plugin
87 needs to use that function, I generally only export things after it's
88 clear a lot of plugins will need them.
89 * Just looked through the inline plugin. So if I want to use pagetitle in my code,
90 I have to use the IkiWiki package instead of IkiWiki::Plugin::Wikiwyg? Or would a
91 better approach be to just copy that function into the Wikiwyg plugin?
92 * You can just call `IkiWiki::pagetitle()`.
93 > Note: pagetitle is now exported.
95 wikiwyg.tar.gz
97 * Would it be possible to provide a diff between wikiwyg upstream and any
98 modifications you made to its files? I'm not sure which version you used,
99 so I'm seeing changes in diffing that I'm unsure if you made..
100 * <http://ikiwiki.xbaud.com/JavaScript_Diffs.tar.gz>, also emailed them to you
101 in case my internet goes down.
102 * Could you redo that with diff -u plz?
103 * Link is updated
104 * If the files aren't modified, would it be better for users to get them
105 from the wikiwgy upstream, instead of including them in the plugin? (If so,
106 they'd go in their own Debian package..)
107 * The files *are* modified, but I doubt it will make a difference. There have
108 been no updates to Wikiwyg since 5/30/07 so I'm pretty sure it's unmaintained
109 now. Showdown is the same case, they haven't changed anything since SoC began.
110 I could separate them diff's though if you feel it is worth it.
111 * Well, from a packaging perspective, the question is whether some
112 other package might want to use the wikiwyg/showdown javascript
113 files. And whether your mods might break that. If the answers to
114 these questions are yes and no, then it would make sense to package
115 them as standalone packages rather than embedding them in ikiwiki.
117 misc:
119 * What are your thoughts on handling plugins? Just make preview do a
120 server-side callback?
121 * That is an option, however I was trying to avoid that due to bandwidth, cpu time
122 concerns (Two reasons I really like IkiWiki). I was planning on just manually
123 implementing some of the easier ones (such as img), however I'm still trying to
124 think of a way for the more complex ones.
125 * It just seems like it would never be able to support everything,
126 and would mean reimplementing stuff in javscript and would constantly
127 need to be kept up to date. Ikiwiki's preview is actually pretty
128 fast, the only real overhead being the cgi call.
129 * How do I configure it to only support whole-page editing with wikiwyg and
130 not insert the javascript into html pages?
131 * There currently is no option to do that, however it is a 2 line change that I'll work
132 on after I finish typing this.
133 * When editing a whole page with wikiwyg, I think it would be good to keep
134 the save, preview, cancel buttons at the bottom like they are in a
135 regular page edit. Also the comments box. Kind of a least suprise thing, so that enabling
136 wikiwyg for whole-page editing basically just changes how the edit box
137 behaves and keeps the rest of the behavior the same. And I think the preview
138 button should show a preview rendered server-side, like with a regular edit,
139 since such a preview is able to support all plugins.
140 * That's probably a good idea ;)
142 Everything else looks fine and ready for merging. If, that is, you think
143 I should include the plugin with all of its java code in ikiwiki. Thoughts?
145 --[[Joey]]
147 I'll start working on the changes... Let me know if you find anything else
148 that needs to be changed. I'd be honored to have my code merged with IkiWiki :)
150 --[[TaylorKillian]]
152 I wonder if you've had a chance to make any of the remaining changes above?
153 Even just some of the smaller changes would be much easier for you to
154 do than for me, and it would be nice to get them sorted out before I
155 merge it into ikiwiki. --[[Joey]
157 None of the links for the WYSIWYG editor work anymore. Does anyone have an up to date link?
158 Thanks, [[Greg]]
160 > There's a branch in [[git]] for the wikiwyg stuff, which includes
161 > the latest version I sucked in from TaylorKillian's svn repository before
162 > it went offline. Disapponted that nothing seems to be moving here.
163 > --[[Joey]]
165 >> How far from ready did this seem to be at that point? I find it a bit unclear
166 >> in the above discussion what was completed and what remained. Also, to recover the
167 >> wikiwyg-specific stuff from git, it looks like I'd need to ask git for
168 >> a diff between the wikiwyg branch and its branch point; is there a nice way to do
169 >> that with gitweb, or would I need to install a full-fledged git client? --Chapman Flack
171 >>> I think that the largest missing thing was support for using ikiwiki
172 >>> to render page previews.
173 >>>
174 >>> Erm.. I seem to have screwed up the creation or pushing out of the
175 >>> wikiwyg branch. It doesn't seem to have any of the wikiwyg changes in
176 >>> it, and at this point, I don't know where to find them anymore! Damn,
177 >>> damn, damn. I suspect I did that right when I was learning git, and
178 >>> screwed up pushing the branch. :-( --[[Joey]]