1 2012-01-06T00:31:32 *** MattMaker
2 2012-01-06T01:05:52 *** RogerHaase
3 2012-01-06T02:09:27 *** MattMaker
4 2012-01-06T02:09:38 *** MattMaker
5 2012-01-06T02:24:30 *** qxcv
6 2012-01-06T03:15:50 *** Dragooon
7 2012-01-06T03:27:13 <Dragooon> anyone here?
8 2012-01-06T03:28:08 <Dragooon> ThomasWaldmann: there?
9 2012-01-06T03:31:47 <ThomasWaldmann> Dragooon: yes
10 2012-01-06T03:32:09 <Dragooon> ThomasWaldmann: My code does have CSS in it, it should show the removed block in red and added block in green
11 2012-01-06T03:35:55 <ThomasWaldmann> hm, somehow it doesn't
12 2012-01-06T03:36:08 <ThomasWaldmann> using ff here
13 2012-01-06T03:36:37 <Dragooon> ff reads atom feeds natively?
14 2012-01-06T03:36:55 <Dragooon> let me check
15 2012-01-06T03:38:47 <ThomasWaldmann> i just looked at the feed url
16 2012-01-06T03:39:32 <Dragooon> I see
17 2012-01-06T03:39:57 <ThomasWaldmann> btw, for the global feed it displays "My MoinMoin - " while for a page it displays "My MoinMoin - Home"
18 2012-01-06T03:41:59 <Dragooon> Oh, that is FF imposing it's own stylesheet on atom feeds
19 2012-01-06T03:42:03 <Dragooon> Can't do much about that
20 2012-01-06T03:42:14 <Dragooon> On my atom reader I can see the feeds fine
21 2012-01-06T03:44:24 <ThomasWaldmann> which one do you use?
22 2012-01-06T03:44:40 <Dragooon> I'm on Mac, NetNewsWire Lite
23 2012-01-06T03:44:58 <Dragooon> I can slightly improve the styling still for such cases
24 2012-01-06T03:45:39 <Dragooon> But not by much
25 2012-01-06T03:52:39 <Dragooon> ThomasWaldmann: Pushed a new revision, please try that. That's probably the best FF or any other reader supports without stylesheets
26 2012-01-06T03:52:59 <Dragooon> IF you want, I can give you screenshots before/after. That's required by my task anyway
27 2012-01-06T03:54:43 <izibi> Dragooon: try adding !important and see what happens
28 2012-01-06T03:55:11 <Dragooon> Nothing
29 2012-01-06T03:55:36 <izibi> and make sure it looks good on google reader :P good luck testing that :D
30 2012-01-06T03:57:42 <Dragooon> Oh crap
31 2012-01-06T03:58:12 <izibi> ;)
32 2012-01-06T03:59:40 <ThomasWaldmann> hmm, tried with liferea, but somehow it has troubles with the feed
33 2012-01-06T04:01:29 <Dragooon> Let's try that then
34 2012-01-06T04:02:56 <ThomasWaldmann> the colours are very light, one almost can't see them on some displays
35 2012-01-06T04:04:16 <Dragooon> Took them from moinmoin's diff display
36 2012-01-06T04:04:57 <ThomasWaldmann> really? strange.
37 2012-01-06T04:05:49 <Dragooon> What troubles does it have with the feed?
38 2012-01-06T04:05:49 <ThomasWaldmann> please make the first revision display style a bit more similar to the diff style
39 2012-01-06T04:06:34 <ThomasWaldmann> like displaying "Revision: xxxxx (first, full text shown)" or something similar in similar style
40 2012-01-06T04:06:43 <ThomasWaldmann> had a typo, works now
41 2012-01-06T04:07:35 <Dragooon> lol
42 2012-01-06T04:09:59 <Dragooon> ThomasWaldmann: For first revision, should I serve the content as HTML or send raw html?
43 2012-01-06T04:11:09 *** moin6
44 2012-01-06T04:11:25 <ThomasWaldmann> playing with that and seeing what's better was the intended task :)
45 2012-01-06T04:12:44 <Dragooon> What do you mean?
46 2012-01-06T04:12:50 <Dragooon> Oh okay
47 2012-01-06T04:14:57 <ThomasWaldmann> btw, also compare the web diff display with the atom diff display when adding/removing a word in a bigger paragraph.
48 2012-01-06T04:15:19 <ThomasWaldmann> on the web diff i see the word change clearly, but not on the feed diff display
49 2012-01-06T04:15:52 <Dragooon> For that I'd need to modify diff_html to suite this
50 2012-01-06T04:16:02 <Dragooon> Maybe not, let me check
51 2012-01-06T04:16:26 <ThomasWaldmann> maybe check how the web display does it
52 2012-01-06T04:17:00 <Dragooon> Web display probably uses css stylesheets
53 2012-01-06T04:17:01 <ThomasWaldmann> also, i think we need to think about the "no comment" display, this somehow looks annoying
54 2012-01-06T04:17:14 <ThomasWaldmann> we already discussed that, i know
55 2012-01-06T04:18:07 <ThomasWaldmann> btw, also please try some non-text item modification. what happens? what should happen?
56 2012-01-06T04:19:33 <Dragooon> how can I get the revision ID from rev?
57 2012-01-06T04:19:36 <Dragooon> rev.meta.get(ID, '')
58 2012-01-06T04:19:37 <Dragooon> ?
59 2012-01-06T04:25:43 <Dragooon> ThomasWaldmann: I've taken care of the rest, what should I do with title?
60 2012-01-06T04:27:14 <ThomasWaldmann> think about it
61 2012-01-06T04:27:30 <Dragooon> I can leave it empty
62 2012-01-06T04:27:37 <Dragooon> Or I can add the revision : <hash> instead
63 2012-01-06T04:27:58 <Dragooon> I think second one sounds better
64 2012-01-06T04:33:05 <Dragooon> ThomasWaldmann: Please see now
65 2012-01-06T04:33:11 <Dragooon> Just pushed another revision
66 2012-01-06T04:37:47 <ThomasWaldmann> no incoming changes
67 2012-01-06T04:38:54 <Dragooon> Sorry, try now
68 2012-01-06T04:42:01 <moin6> ThomasWaldmann: When you have a minute, can I ask you about the Google Code-In task for unused CSS?
69 2012-01-06T04:44:33 *** xjjk
70 2012-01-06T04:57:56 <Dragooon> ThomasWaldmann: there?
71 2012-01-06T05:03:38 <ThomasWaldmann> ok, got changes now
72 2012-01-06T05:04:49 <ThomasWaldmann> diff display much better now
73 2012-01-06T05:05:14 <ThomasWaldmann> the first rev display shows raw html now
74 2012-01-06T05:05:25 <Dragooon> Yeah, I made it like normal diff display
75 2012-01-06T05:05:38 <ThomasWaldmann> <div xmlns="http://www.w3.org/1999/xhtml"><p>foo</p></div>
76 2012-01-06T05:06:02 <ThomasWaldmann> i dont think normal users would appreciate :D
77 2012-01-06T05:06:12 <ThomasWaldmann> moin6: just ask
78 2012-01-06T05:06:21 <Dragooon> I'll revert that, what about title?
79 2012-01-06T05:08:41 <Dragooon> ThomasWaldmann: ^
80 2012-01-06T05:10:09 <ThomasWaldmann> maybe if we do not want to repeat the page name, the editor name/ip + comment would be the best
81 2012-01-06T05:10:31 <ThomasWaldmann> so, if there is no comment, you see at least who did the change
82 2012-01-06T05:10:46 <Dragooon> okay, 2 minutes
83 2012-01-06T05:11:17 <moin6> I want to make sure I am understanding the task correctly; the final deliverable is a bitbucket issue that highlights what I find after I analyze which CSS selectors are not being used or are being referenced but are not actually in the CSS file? Correct
84 2012-01-06T05:13:08 <ThomasWaldmann> moin6: give the task url please
85 2012-01-06T05:13:27 <moin6> http://www.google-melange.com/gci/task/view/google/gci2011/7159202
86 2012-01-06T05:15:28 <ThomasWaldmann> moin6: yes
87 2012-01-06T05:16:09 <ThomasWaldmann> (i think you understood correctly)
88 2012-01-06T05:16:50 <moin6> Ok. I found the CSS files in the repository. What other source files do I need (Python, Html, Javascript)?
89 2012-01-06T05:19:03 <Dragooon> ThomasWaldmann: How can I get the author's name from the dict returned in get_author_info?
90 2012-01-06T05:19:24 <Dragooon> get_editor_info*
91 2012-01-06T05:20:24 <Dragooon> Wait, nvm, figured it out
92 2012-01-06T05:21:07 <ThomasWaldmann> moin6: you are working on the moin-2.0 repo, right?
93 2012-01-06T05:22:15 <ThomasWaldmann> moin6: in moin2, most references should come from the templates, but you task is a systematic review, that includes py and js also
94 2012-01-06T05:22:55 <moin6> yes. I am working on the moin-2.0 repo
95 2012-01-06T05:23:03 <Dragooon> ThomasWaldmann: Pushed another revision
96 2012-01-06T05:23:35 <moin6> So should I be checking all files in the entire repository
97 2012-01-06T05:23:37 <moin6> ?
98 2012-01-06T05:25:25 <ThomasWaldmann> yes
99 2012-01-06T05:25:41 <ThomasWaldmann> you should use some sane mechanism to do that
100 2012-01-06T05:26:02 <ThomasWaldmann> e.g. some tool or some good regex or ... (find out)
101 2012-01-06T05:26:41 <moin6> Is there a specific format you want see in the final submission?
102 2012-01-06T05:28:26 <ThomasWaldmann> no, make something useful :)
103 2012-01-06T05:29:27 <ThomasWaldmann> the most important point of this task is: a) systematic b) both directions
104 2012-01-06T05:30:16 <moin6> Ok. I'll get to work on that.
105 2012-01-06T05:31:15 *** MattMaker
106 2012-01-06T05:33:01 *** MattMaker
107 2012-01-06T05:33:31 * ThomasWaldmann assigned the task
108 2012-01-06T05:33:48 <Dragooon> Is the title okay now?
109 2012-01-06T05:35:35 <ThomasWaldmann> well, currently it just shows 0.0.0.0 but that is not your fault :)
110 2012-01-06T05:36:12 <Dragooon> Yeah, can't do much about it
111 2012-01-06T05:37:15 <ThomasWaldmann> ok, I'll head to bed now. maybe investigate other content-types.
112 2012-01-06T05:37:58 <Dragooon> Darn, I was hoping for approval today
113 2012-01-06T05:43:53 *** bilal
114 2012-01-06T05:44:04 <ThomasWaldmann> well, this is a hard one :)
115 2012-01-06T05:44:19 <Dragooon> I've done hard tasks in a single day :)
116 2012-01-06T05:44:58 <ThomasWaldmann> that maybe was a fault of the org then. or a different kind of hardness.
117 2012-01-06T05:45:34 * ThomasWaldmann isn't too happy with that sort of classification.
118 2012-01-06T05:45:48 <Dragooon> I am not saying that it's fault of anybody
119 2012-01-06T05:45:53 <Dragooon> It's just that 5 days for a single task is too much
120 2012-01-06T05:46:50 <ThomasWaldmann> well, you can optimize that (or try at least)
121 2012-01-06T05:47:03 <Dragooon> I'm trying, just need a mentor for a while
122 2012-01-06T05:47:38 <Dragooon> I mean this task is almost done, the other content-types work although they don't show diff for binaries
123 2012-01-06T05:47:56 <Dragooon> If I ask a different mentor he'll have to review it from the beginning
124 2012-01-06T05:47:59 <ThomasWaldmann> what do they show?
125 2012-01-06T05:48:11 <Dragooon> Images show that the content has different data
126 2012-01-06T05:48:18 <Dragooon> So do the most of other binaries
127 2012-01-06T05:48:32 <Dragooon> text types work with proper diffs
128 2012-01-06T05:48:53 <ThomasWaldmann> well, that is ok for web ui but not that useful for feed
129 2012-01-06T05:49:13 <Dragooon> Can't do much, sending full images would probably not work in most cases
130 2012-01-06T05:49:29 <ThomasWaldmann> send a link maybe?
131 2012-01-06T05:50:16 <Dragooon> Link is already sent
132 2012-01-06T05:50:31 <Dragooon> In link attribute of the feed
133 2012-01-06T05:52:29 <ThomasWaldmann> btw, for the global feed ( /+feed/atom ) i do not see the item name
134 2012-01-06T05:53:06 <Dragooon> Hm...
135 2012-01-06T05:53:17 <ThomasWaldmann> and sending a picture seems to work, as it did just that for the first revision
136 2012-01-06T05:54:21 <Dragooon> Sending 2 pictures?
137 2012-01-06T05:54:34 <ThomasWaldmann> (at least for liferea, i don't know whether all readers support that)
138 2012-01-06T05:54:53 <ThomasWaldmann> no, for that case i think it is better to just send the new one
139 2012-01-06T05:54:53 <Dragooon> Mine seems to be rejecting pics
140 2012-01-06T05:55:13 <ThomasWaldmann> or a link if you think that does not work
141 2012-01-06T05:56:58 <ThomasWaldmann> hmm, in liferea i need to click on the 0.0.0.0 to view the item
142 2012-01-06T05:57:18 <ThomasWaldmann> not very intuitive right now
143 2012-01-06T05:57:41 <Dragooon> That's probably due to liferea's rendering
144 2012-01-06T05:57:53 <ThomasWaldmann> well, it is the title
145 2012-01-06T05:58:15 <Dragooon> Ah
146 2012-01-06T05:58:27 <ThomasWaldmann> or "headline"
147 2012-01-06T05:59:24 <ThomasWaldmann> anyway, need sleep. please do some practical experimentation and see how the current output matches user expectations. if you find mismatches, try to fix it.
148 2012-01-06T06:23:00 *** moin6
149 2012-01-06T06:30:07 *** xjjk
150 2012-01-06T06:31:31 *** MattMaker
151 2012-01-06T06:34:15 *** MattMaker
152 2012-01-06T07:01:21 *** moin4
153 2012-01-06T07:24:20 *** moin4
154 2012-01-06T08:56:55 <Dragooon> dreimark: there?
155 2012-01-06T09:20:25 <dreimark> moin
156 2012-01-06T09:20:40 <dreimark> Dragooon: it looks on ff like this now http://test.moinmo.in/feed1
157 2012-01-06T09:21:04 <dreimark> why is it twice "2"
158 2012-01-06T09:50:39 <Dragooon> Hello
159 2012-01-06T09:50:43 <Dragooon> dreimark: there?
160 2012-01-06T10:11:37 <Dragooon> I have pushed a new revision to repository that handles binary and other data types much better
161 2012-01-06T10:11:40 <Dragooon> Please have a look at that
162 2012-01-06T10:16:16 *** greg_f
163 2012-01-06T12:41:05 <dreimark> Dragooon: i have still 2: 2:
164 2012-01-06T12:41:27 <Dragooon> dreimark: those are line numbers
165 2012-01-06T12:43:11 <dreimark> ah
166 2012-01-06T12:43:34 <dreimark> if you have image revisions, do you see a diff of those?
167 2012-01-06T12:43:35 <Dragooon> I would've added line but they were taking fairly limited space
168 2012-01-06T12:43:38 <Dragooon> No
169 2012-01-06T12:43:50 <Dragooon> It shows latest image with info of size and revision hash
170 2012-01-06T12:44:05 <Dragooon> Generating image diff would be fairly resource intensive IMO
171 2012-01-06T12:45:24 <dreimark> no
172 2012-01-06T12:45:33 <Dragooon> really?
173 2012-01-06T12:45:38 <dreimark> have you tried? we have that on the normal diff
174 2012-01-06T12:45:40 <dreimark> of images
175 2012-01-06T12:45:47 <Dragooon> No?
176 2012-01-06T12:45:52 <dreimark> it is fast
177 2012-01-06T12:45:57 <Dragooon> There is no diff of image in moin2
178 2012-01-06T12:46:02 <dreimark> it is
179 2012-01-06T12:46:08 <dreimark> you need to install PIL
180 2012-01-06T12:46:18 <Dragooon> Oh
181 2012-01-06T12:46:35 <Dragooon> I can implement that
182 2012-01-06T12:46:42 <Dragooon> Shouldn't be hard since it already exists
183 2012-01-06T12:46:45 <dreimark> pip has currently an issue because it did not install extras_require
184 2012-01-06T12:47:07 <dreimark> the next release has that fixed
185 2012-01-06T12:47:31 <Dragooon> Can you test for me? I'll commit in a minute
186 2012-01-06T12:47:36 <dreimark> ok
187 2012-01-06T12:51:39 <Dragooon> Just pushed, hopefully it'll work
188 2012-01-06T12:53:09 <Dragooon> dreimark: ^
189 2012-01-06T12:56:42 <dreimark> it did not show the diff in the feed
190 2012-01-06T12:56:46 <dreimark> another question
191 2012-01-06T12:56:59 <dreimark> is it usual that i have a feed per each change
192 2012-01-06T12:57:07 <Dragooon> Yea
193 2012-01-06T12:57:19 <Dragooon> Hm...
194 2012-01-06T12:57:26 <Dragooon> You have PIL installed?
195 2012-01-06T12:57:46 <dreimark> sure it don't work only in the feed
196 2012-01-06T12:58:40 <Dragooon> let me try installing it then
197 2012-01-06T12:58:41 <dreimark> it shows the new image
198 2012-01-06T12:59:24 <Dragooon> how do I install PIL?
199 2012-01-06T13:00:29 <dreimark> pip install PIL
200 2012-01-06T13:04:24 *** qxcv
201 2012-01-06T13:06:06 <Dragooon> Oh my god…what a stupid mistake
202 2012-01-06T13:07:54 <Dragooon> dreimark: Pushed another revision, works for me locally
203 2012-01-06T13:16:48 *** qxcv
204 2012-01-06T13:25:19 <Dragooon> dreimark: are you still here?
205 2012-01-06T13:26:25 <dreimark> yes me looks soon
206 2012-01-06T13:26:47 <Dragooon> let me know when
207 2012-01-06T13:29:53 <dreimark> don't work. i try always ff and liefrea
208 2012-01-06T13:30:15 * dreimark pulls again
209 2012-01-06T13:31:00 <dreimark> now it works in ff
210 2012-01-06T13:31:48 <Dragooon> :D
211 2012-01-06T13:31:49 <dreimark> it works also on liferea
212 2012-01-06T13:32:24 <Dragooon> so, is the feed okay?
213 2012-01-06T13:34:52 <dreimark> that part works now
214 2012-01-06T13:35:04 <dreimark> have you -k test_sourceode ?
215 2012-01-06T13:35:15 <dreimark> test_sourcecode
216 2012-01-06T13:35:32 <Dragooon> Not as of late, if everything else is fine I'll clean that up
217 2012-01-06T13:35:43 <dreimark> you have pep8 issues
218 2012-01-06T13:35:58 <dreimark> usually you do tah before you commit
219 2012-01-06T13:36:09 <Dragooon> Yes, sorry.
220 2012-01-06T13:38:22 *** dave_largo
221 2012-01-06T13:38:51 <Dragooon> dreimark: Anything else that's wrong? I've fixed test_sourcecode errors
222 2012-01-06T13:39:23 <dreimark> i have to look into the source
223 2012-01-06T13:39:38 <dreimark> i have no idea yet how it looks like on a smaller device
224 2012-01-06T13:40:32 <dreimark> as a user i likly not been interested in gettin on top as content Revision and size
225 2012-01-06T13:40:37 <dreimark> I like to see content
226 2012-01-06T13:41:55 <dreimark> is there an other place possible
227 2012-01-06T13:42:00 <Dragooon> I see, so remove them or move them to the end/
228 2012-01-06T13:42:26 <dreimark> move it somewhere, or can it be hidden / toggled?
229 2012-01-06T13:42:46 <Dragooon> can't be toggled since many feed readers don't allow JS
230 2012-01-06T13:42:52 <Dragooon> I can remove it
231 2012-01-06T13:43:01 <dreimark> move it to the end
232 2012-01-06T13:46:36 <Dragooon> dreimark: Pushed changes
233 2012-01-06T13:49:40 <qxcv> ThomasWaldmann: reading your comments on codereview about using mutable kwargs, do you know of a more elegant solution?
234 2012-01-06T14:02:02 <Dragooon> dreimark: there?
235 2012-01-06T14:10:28 <dreimark> sure but busy too
236 2012-01-06T14:10:40 <Dragooon> Oh sorry
237 2012-01-06T14:12:55 <dreimark> Dragooon: try out drawing items
238 2012-01-06T14:13:51 <Dragooon> Oh, that won't be pretty
239 2012-01-06T14:13:53 <dreimark> or all kind of items from the ui with creation and one modification
240 2012-01-06T14:13:59 <qxcv> ThomasWaldmann: also, I realised the code in my patch would cause a race condition, so I definitely have to figure out a better way of passing instance_path into the config :P
241 2012-01-06T14:14:25 <Dragooon> I tried audio, video, images and text ones. Didn't try drawable
242 2012-01-06T14:37:24 <Dragooon> dreimark: there's a bug in drawable items where the item will only render the latest item
243 2012-01-06T14:37:54 <Dragooon> attest there seems to be
244 2012-01-06T14:37:57 <Dragooon> atlest*
245 2012-01-06T14:37:59 <Dragooon> atlest*
246 2012-01-06T14:38:02 <Dragooon> atleast*
247 2012-01-06T14:42:58 <Dragooon> dreimark: Committed a fix to that problem, everything else seems to be working fine
248 2012-01-06T14:43:02 <Dragooon> Please test when you have time
249 2012-01-06T14:52:17 *** qxcv
250 2012-01-06T15:08:06 <Dragooon> dreimark: I'll be away for a while, if you have any comments leave a message in IRC
251 2012-01-06T15:08:07 <Dragooon> Thanks
252 2012-01-06T16:31:06 <ThomasWaldmann> Dragooon: + if rev_comment is not '':
253 2012-01-06T16:31:39 <ThomasWaldmann> that this is working is rather luck (or a implementation detail)
254 2012-01-06T16:32:02 <Dragooon> what should that be then?
255 2012-01-06T16:32:09 <ThomasWaldmann> what you likely want is "if rev_comment:"
256 2012-01-06T16:32:33 <Dragooon> ah stupid me
257 2012-01-06T16:32:41 <ThomasWaldmann> "is" is checking for object identity, but you rather want to check for value
258 2012-01-06T16:34:09 <Dragooon> okay, fixed
259 2012-01-06T16:34:50 <ThomasWaldmann> then, if you use x.format(), make sure that x is unicode, e.g. u"...."
260 2012-01-06T16:34:51 <Dragooon> I've also taken care of binary items as well as image diffs
261 2012-01-06T16:35:04 <Dragooon> Ah okay
262 2012-01-06T16:35:36 <ThomasWaldmann> otherwise you'll get unicode errors when some parameter is unicode and can't get encoded to str using the ascii encoder
263 2012-01-06T16:35:38 <Dragooon> BTW, I also fixed a bug in which drawable would only show the latest revision's content
264 2012-01-06T16:35:55 <ThomasWaldmann> great :)
265 2012-01-06T16:36:19 <Dragooon> not sure if I can claim for that, since I can't figure out a way to make an unit test for it
266 2012-01-06T16:36:48 <ThomasWaldmann> we'll see afterwards
267 2012-01-06T16:36:55 <Dragooon> yeah okay
268 2012-01-06T16:38:53 <ThomasWaldmann> then, why did you change xhtml to html?
269 2012-01-06T16:39:40 <Dragooon> From what I remember, it was causing some problem
270 2012-01-06T16:39:44 <Dragooon> But I cannot remember what
271 2012-01-06T16:40:32 <ThomasWaldmann> strange. can you try?
272 2012-01-06T16:40:41 <ThomasWaldmann> because i think we give xhtml, right?
273 2012-01-06T16:41:06 <ThomasWaldmann> MoinMoin/apps/feed/views.py you removed the last line of that file (which was empty), please add it again
274 2012-01-06T16:41:23 <Dragooon> Both, xhtml and html are valid
275 2012-01-06T16:41:25 <ThomasWaldmann> we usually want files to end with an empty line, because otherwise diff likes to complain
276 2012-01-06T16:41:45 <Dragooon> Hm…I have an empty line
277 2012-01-06T16:42:01 <ThomasWaldmann> ok, maybe there were two
278 2012-01-06T16:43:04 <ThomasWaldmann> + def _render_data_diff(self, oldrev, newrev):
279 2012-01-06T16:43:04 <ThomasWaldmann> + print 'test'
280 2012-01-06T16:43:04 <ThomasWaldmann> + print self._internal_representation()
281 2012-01-06T16:43:13 <ThomasWaldmann> that somehow looks like debug code
282 2012-01-06T16:44:06 <Dragooon> Oh man
283 2012-01-06T16:47:14 <ThomasWaldmann> + drawing_url = url_for('frontend.get_item', item_name=item_name, member='drawing.draw', rev=self.rev.revid)
284 2012-01-06T16:47:17 <ThomasWaldmann> + png_url = url_for('frontend.get_item', item_name=item_name, member='drawing.png', rev=self.rev.revid)
285 2012-01-06T16:47:21 <ThomasWaldmann> is that the bugfix you meant?
286 2012-01-06T16:49:21 <Dragooon> Yes
287 2012-01-06T16:50:44 <ThomasWaldmann> +{% macro safe_style() %}
288 2012-01-06T16:50:52 <ThomasWaldmann> why is that named as it is?
289 2012-01-06T16:51:11 <Dragooon> it contains styles to make rendering a bit safer for html feed viewers
290 2012-01-06T16:51:31 <Dragooon> images can blow out of propotion and screw up if not specified
291 2012-01-06T16:51:59 <Dragooon> So can tables
292 2012-01-06T16:52:26 <Dragooon> unfortunately ff disregards anything starting with 'style'
293 2012-01-06T16:52:37 <ThomasWaldmann> well, add a comment to the source
294 2012-01-06T16:52:51 <ThomasWaldmann> also, maybe include "feed" or "atom" to the macro name
295 2012-01-06T16:53:24 <ThomasWaldmann> also, why do you put the macro call into <strong>...</strong> without any printable content?
296 2012-01-06T16:53:45 <Dragooon> where?
297 2012-01-06T16:53:59 <Dragooon> Oh, to bold it
298 2012-01-06T16:54:58 <ThomasWaldmann> to bold what?
299 2012-01-06T16:55:11 <Dragooon> To bold the meta contents
300 2012-01-06T16:55:49 <ThomasWaldmann> i don't see contents there
301 2012-01-06T16:56:37 <Dragooon> It should show revision content
302 2012-01-06T16:59:11 <ThomasWaldmann> http://paste.pocoo.org/show/530927/
303 2012-01-06T17:21:08 <ThomasWaldmann> Dragooon: ^ there is no content
304 2012-01-06T17:25:47 <Dragooon> ThomasWaldmann: Oh, okay. Fixed
305 2012-01-06T17:25:56 <Dragooon> Sorry, went away for dinner
306 2012-01-06T17:30:15 <ThomasWaldmann> + <p style="font-size: 14px">...{{ comment }}</p> <br /> {{ content }}
307 2012-01-06T17:30:23 <ThomasWaldmann> use em please, not px
308 2012-01-06T17:31:36 <ThomasWaldmann> \ No newline at end of file
309 2012-01-06T17:31:54 <ThomasWaldmann> (atom.html - this is the diff warning i mentioned above)
310 2012-01-06T17:32:26 <Dragooon> okay
311 2012-01-06T17:34:06 <ThomasWaldmann> same in atom.xslt
312 2012-01-06T17:37:52 <Dragooon> okay
313 2012-01-06T17:38:19 <Dragooon> test_sourcecode should test for that, no?
314 2012-01-06T17:38:39 <ThomasWaldmann> should yeah. not sure it does.
315 2012-01-06T17:39:03 <ThomasWaldmann> but one easily sees when reading hg diff :)
316 2012-01-06T17:39:09 <Dragooon> Yeah
317 2012-01-06T17:41:17 <Dragooon> Is the behavior itself fine?
318 2012-01-06T17:48:04 <ThomasWaldmann> i only tested wiki markup and png item
319 2012-01-06T17:48:12 <ThomasWaldmann> looks ok
320 2012-01-06T17:49:16 <Dragooon> okay, let me know if you find any problems. I'll commit at once
321 2012-01-06T17:50:29 <ThomasWaldmann> well, i suggest you make changeset from the above changes and push
322 2012-01-06T17:50:41 <ThomasWaldmann> so i can look at an updated overall diff
323 2012-01-06T17:52:07 <Dragooon> Pushed
324 2012-01-06T17:59:19 <ThomasWaldmann> btw, did you check caching behaviour of that code?
325 2012-01-06T17:59:43 <Dragooon> Not really
326 2012-01-06T17:59:48 <Dragooon> Don't know how exactly it works
327 2012-01-06T18:00:17 <Dragooon> But reading the code again, it should be fine
328 2012-01-06T18:00:30 <ThomasWaldmann> + def _render_data_diff(self, oldrev, newrev):
329 2012-01-06T18:00:31 <ThomasWaldmann> + print self._internal_representation()
330 2012-01-06T18:00:41 <ThomasWaldmann> you want to keep that?
331 2012-01-06T18:00:44 <Dragooon> no
332 2012-01-06T18:00:48 *** greg_f
333 2012-01-06T18:02:04 <Dragooon> should I push?
334 2012-01-06T18:02:23 <ThomasWaldmann> wait a sec, i am within doing a 2nd pass
335 2012-01-06T18:02:31 <Dragooon> okay
336 2012-01-06T18:03:21 <ThomasWaldmann> there is still no comment in safe_style and you neither changed its name
337 2012-01-06T18:06:05 <Dragooon> done
338 2012-01-06T18:06:22 <ThomasWaldmann> those classes you use in atom.html, like moin-diff etc. - do they do something?
339 2012-01-06T18:06:48 <Dragooon> Nope, just kept them for semantics
340 2012-01-06T18:10:38 <ThomasWaldmann> ok, commit / push
341 2012-01-06T18:11:28 <Dragooon> Done
342 2012-01-06T18:13:11 <CIA-59> Shitiz Garg <mail@dragooon.net> default * 1196:8905bb8bcb5f 2.0/MoinMoin/util/diff_html.py: Use unicode while generating diff for HTML
343 2012-01-06T18:13:12 <CIA-59> Shitiz Garg <mail@dragooon.net> default * 1197:98ff5e7a0bec 2.0/MoinMoin/ (5 files in 4 dirs): Improved atom feed for page history
344 2012-01-06T18:13:13 <CIA-59> Shitiz Garg <mail@dragooon.net> default * 1200:302e1a86f494 2.0/MoinMoin/items/_tests/test_Item.py: Add unit test for _render_data_diff
345 2012-01-06T18:13:17 <CIA-59> Shitiz Garg <mail@dragooon.net> default * 1198:2aa4f4ec6a94 2.0/MoinMoin/ (apps/feed/views.py items/__init__.py templates/atom.html): Minor fixes for atom feed
346 2012-01-06T18:13:18 <CIA-59> Shitiz Garg <mail@dragooon.net> default * 1199:a4ce283b0ca2 2.0/MoinMoin/ (3 files in 3 dirs): Text::_render_data_diff should return unicode in keeping it consistent with _render_data_diff_atom
347 2012-01-06T18:13:22 <CIA-59> Shitiz Garg <mail@dragooon.net> default * 1205:b76652381200 2.0/MoinMoin/ (3 files in 2 dirs): Slight improvement in diff presentation, also removed trailing dash for global feed
348 2012-01-06T18:13:22 <CIA-59> Shitiz Garg <mail@dragooon.net> default * 1206:19e7e10e0dd0 2.0/MoinMoin/ (3 files in 2 dirs): Some more improvement in diff presentation, also changed revisions with empty comment's titles.
349 2012-01-06T18:13:23 <CIA-59> Shitiz Garg <mail@dragooon.net> default * 1209:c0eaed311e0e 2.0/MoinMoin/items/__init__.py: Try using PIL for image diffs
350 2012-01-06T18:13:24 <CIA-59> Shitiz Garg <mail@dragooon.net> default * 1203:057a7872cf64 2.0/MoinMoin/items/_tests/test_Item.py: Add check for diff_html return types
351 2012-01-06T18:13:26 <CIA-59> Shitiz Garg <mail@dragooon.net> default * 1212:98d0b7dfaf1a 2.0/MoinMoin/items/__init__.py: Fix drawable items would always render the latest revision
352 2012-01-06T18:13:27 <CIA-59> Shitiz Garg <mail@dragooon.net> default * 1214:8d547ec39a76 2.0/MoinMoin/ (items/__init__.py templates/atom.html): Cosmetic changes
353 2012-01-06T18:13:28 <CIA-59> Shitiz Garg <mail@dragooon.net> default * 1211:e63c9c8d0ad9 2.0/MoinMoin/ (3 files in 2 dirs): Move revision info to the end
354 2012-01-06T18:13:29 <CIA-59> Shitiz Garg <mail@dragooon.net> default * 1213:0edcdc5e7e91 2.0/MoinMoin/ (5 files in 4 dirs): Treat feed_title as unicode, also some cosmetic fixes
355 2012-01-06T18:13:30 <CIA-59> Shitiz Garg <mail@dragooon.net> default * 1204:608caaa27937 2.0/MoinMoin/ (apps/frontend/views.py templates/diff.html): Fix diff.hmtl
356 2012-01-06T18:13:32 <CIA-59> Shitiz Garg <mail@dragooon.net> default * 1208:92d8f0777a9a 2.0/MoinMoin/ (4 files in 3 dirs): Improve handling of other data types in atom feed, also remove anchor tags from line numbers
357 2012-01-06T18:13:33 <CIA-59> Shitiz Garg <mail@dragooon.net> default * 1207:af3fdd4c34e2 2.0/MoinMoin/ (apps/feed/views.py templates/atom.html): Change feed title, also make the first revision show HTML
358 2012-01-06T18:13:34 <CIA-59> Shitiz Garg <mail@dragooon.net> default * 1210:d5c6c9ada478 2.0/MoinMoin/items/__init__.py: Fix incorrect URL while fetching images' diff
359 2012-01-06T18:13:35 <CIA-59> Shitiz Garg <mail@dragooon.net> default * 1201:526c97231401 2.0/MoinMoin/items/_tests/test_Item.py: Fix unit test for _render_data_diff
360 2012-01-06T18:13:36 <CIA-59> Shitiz Garg <mail@dragooon.net> default * 1202:475b3747762c 2.0/MoinMoin/ (items/_tests/test_Item.py util/diff_html.py): Improve unit test for _render_data_diff
361 2012-01-06T18:13:44 <ThomasWaldmann> Dragooon: submit something to melange
362 2012-01-06T18:13:47 * Dragooon should learn to code better
363 2012-01-06T18:14:11 <ThomasWaldmann> well, this is one of the intended side-effects of gci :D
364 2012-01-06T18:14:13 <Dragooon> ThomasWaldmann: Submitted my repo's URL
365 2012-01-06T18:15:08 <Dragooon> http://www.google-melange.com/gci/task/view/google/gci2011/7141270
366 2012-01-06T18:15:21 <Dragooon> Thanks for coping with me
367 2012-01-06T18:15:31 <Dragooon> Hadn't read python before touching this task
368 2012-01-06T18:16:36 <ThomasWaldmann> ok, you want to claim a bugfix easy task for that other bug you fixed?
369 2012-01-06T18:16:58 <Dragooon> the drawable one?
370 2012-01-06T18:17:02 <ThomasWaldmann> yes
371 2012-01-06T18:17:16 <Dragooon> I can't make an unit test for that, no ideas pop into head
372 2012-01-06T18:17:41 <ThomasWaldmann> well, that might be pointless in that case, so it would be ok without
373 2012-01-06T18:17:54 <Dragooon> In that case, sure
374 2012-01-06T18:18:21 <ThomasWaldmann> when claiming, give a precise description of the bug
375 2012-01-06T18:18:38 <ThomasWaldmann> and check the issue tracker if we have it there also
376 2012-01-06T18:19:13 <ThomasWaldmann> (while doing that, also look for atom feed issues there)
377 2012-01-06T18:20:02 <Dragooon> http://www.google-melange.com/gci/task/view/google/gci2011/7235217
378 2012-01-06T18:21:15 <Dragooon> Now such issue atm, and only one for atom feed
379 2012-01-06T18:21:34 <ThomasWaldmann> url?
380 2012-01-06T18:21:43 <Dragooon> https://bitbucket.org/thomaswaldmann/moin-2.0/issue/142/atom-feed-crashes-for-more-than-one
381 2012-01-06T18:21:46 <Dragooon> I created that one
382 2012-01-06T18:22:15 <ThomasWaldmann> ok, but you fixed that, right?
383 2012-01-06T18:22:25 <Dragooon> Yes
384 2012-01-06T18:22:48 <ThomasWaldmann> then put a link to the fix changeset on hg.moinmo.in and close it as resolved
385 2012-01-06T18:22:50 <Dragooon> Marked that task as complete
386 2012-01-06T18:24:57 <Dragooon> https://bitbucket.org/thomaswaldmann/moin-2.0/issue/134/mediawiki-parser-throws-exception-on-file
387 2012-01-06T18:25:03 <Dragooon> ^ What's the difficulty for that task
388 2012-01-06T18:25:04 <Dragooon> ?
389 2012-01-06T18:25:20 <ThomasWaldmann> i suspect medium
390 2012-01-06T18:25:28 <Dragooon> Can I claim?
391 2012-01-06T18:25:39 <ThomasWaldmann> wait a sec
392 2012-01-06T18:26:35 <ThomasWaldmann> look at the atom feed code again, please
393 2012-01-06T18:26:53 <Dragooon> What do you mean?
394 2012-01-06T18:27:07 <ThomasWaldmann> right at the beginning of def atom(): ... there are some comments. review them and update them as required.
395 2012-01-06T18:28:06 <ThomasWaldmann> it looks a bit like you resolved the issues described there, right?
396 2012-01-06T18:30:01 <Dragooon> Yes, I've updated the block comment with a description of what it does now
397 2012-01-06T18:30:04 <Dragooon> Is that fine?
398 2012-01-06T18:30:33 <ThomasWaldmann> ok
399 2012-01-06T18:33:53 <Dragooon> Pushed
400 2012-01-06T18:38:02 <Dragooon> can I claim a medium now/
401 2012-01-06T18:38:04 <Dragooon> ?*
402 2012-01-06T18:43:18 <ThomasWaldmann> did you look at the code whether you can deal with it?
403 2012-01-06T18:43:28 <Dragooon> yes
404 2012-01-06T18:43:36 <CIA-59> Shitiz Garg <mail@dragooon.net> default * 1215:d068e342c04e 2.0/MoinMoin/apps/feed/views.py: Update function's description with information about the current behavior
405 2012-01-06T18:44:35 <ThomasWaldmann> sure, claim it
406 2012-01-06T18:46:21 <Dragooon> ThomasWaldmann: http://codereview.appspot.com/5517058/
407 2012-01-06T18:48:20 <ThomasWaldmann> you can't claim a bugfix task without saying what bug you want to fix (on melange)
408 2012-01-06T18:49:05 <ThomasWaldmann> see "how to do it"
409 2012-01-06T18:49:35 <Dragooon> Okay, replied with a link to the bug
410 2012-01-06T18:51:52 <ThomasWaldmann> ok, you have it
411 2012-01-06T18:52:19 <Dragooon> The code review link is for the fix
412 2012-01-06T18:58:56 <Dragooon> ThomasWaldmann: ^
413 2012-01-06T19:09:28 <ThomasWaldmann> alt should not be empty
414 2012-01-06T19:09:52 <Dragooon> alt?
415 2012-01-06T19:10:11 <ThomasWaldmann> alt attr
416 2012-01-06T19:10:14 <ThomasWaldmann> in your test
417 2012-01-06T19:10:23 <ThomasWaldmann> other than that, test looks ok
418 2012-01-06T19:10:55 <Dragooon> It outputs alt as empty
419 2012-01-06T19:17:53 <dreimark> then it is the next bug
420 2012-01-06T19:18:38 <dreimark> hmm, it can be that is because no alias
421 2012-01-06T19:19:29 <Dragooon> maybe
422 2012-01-06T19:20:12 <Dragooon> Seems so
423 2012-01-06T19:26:18 <ThomasWaldmann> ok, review done
424 2012-01-06T19:27:48 <ThomasWaldmann> just the amount of "link_args.split('|')" there is astonishing
425 2012-01-06T19:29:10 <Dragooon> ok, I'll have a look tomorrow. Thanks
426 2012-01-06T19:30:06 *** Dragooon
427 2012-01-06T20:32:02 *** raignarok
428 2012-01-06T22:30:00 *** dave_largo
429 2012-01-06T23:02:16 <izibi> ThomasWaldmann: http://codereview.appspot.com/5522054
430 2012-01-06T23:02:28 <izibi> this fixes the id/for mismatch
431 2012-01-06T23:03:45 * ThomasWaldmann looks
432 2012-01-06T23:04:22 <izibi> then what about the placeholder? what would you prefer? type="number" (which gives you some nice increase/decrease buttons in some browsers) without placeholder or type="text" with placeholder?
433 2012-01-06T23:06:39 <izibi> but interestingly chromium displays the placeholder for type=number
434 2012-01-06T23:07:11 <ThomasWaldmann> so it happens for radio AND checkbox?
435 2012-01-06T23:08:24 <izibi> yes, but for radio buttons it's usually the case that you have multiple inputs with the same name
436 2012-01-06T23:09:05 <ThomasWaldmann> the fix is a bit ugly
437 2012-01-06T23:09:20 <ThomasWaldmann> you could leave the call as is and fix the problem within the macro
438 2012-01-06T23:09:41 <izibi> if type == checkbox?
439 2012-01-06T23:09:52 <ThomasWaldmann> and also consider all cases where the bug happens, not just checkbox
440 2012-01-06T23:11:51 <izibi> i could imagine that you actually want ids including the value for checkboxes
441 2012-01-06T23:13:25 <ThomasWaldmann> well, discuss with jek, he knows best :)
442 2012-01-06T23:13:50 <izibi> alternatively we could just disable auto_domid and auto_for and just generate them manually in every case
443 2012-01-06T23:14:49 <izibi> i guess that's not related to flatland but on the expected output
444 2012-01-06T23:16:43 <izibi> if you look at the example in the flatland bug
445 2012-01-06T23:17:02 <izibi> in case i'd also include radio buttons, both would get id="f_choice"
446 2012-01-06T23:17:19 <izibi> -> invalid html
447 2012-01-06T23:18:00 <izibi> and for radio buttons you have more then one with the same name, otherwise radio buttons wouldn't make that much sense ;)
448 2012-01-06T23:23:03 <jek> right, that's why the value is on there by default
449 2012-01-06T23:28:11 <izibi> ThomasWaldmann: decision needed ;)
450 2012-01-06T23:30:11 <izibi> "if field_type == 'checkbox'" or "if field_type in ('checkbox', 'radio')"?
451 2012-01-06T23:38:15 <izibi> updated the issue
