1 2011-12-24T00:00:04 <izibi> http://dl.dropbox.com/u/92247/usersettings_ajax_fail.png
2 2011-12-24T00:00:09 <izibi> almost... :D
3 2011-12-24T00:11:21 <izibi> i've uploaded an updated version: http://codereview.appspot.com/5504083/
4 2011-12-24T00:11:34 <izibi> it's not yet finished, but it basically works
5 2011-12-24T00:12:40 <ThomasWaldmann> izibi: if you have js stuff to review, ask someone who knows about js :)
6 2011-12-24T00:22:07 <izibi> would you prefer using a json document or http headers to pass some values?
7 2011-12-24T00:22:31 <izibi> there's no way to handle http redirects in ajax requests
8 2011-12-24T00:23:18 <izibi> so i need to either return a json document that can contain a redirect url or use a custom http header
9 2011-12-24T00:37:00 <ThomasWaldmann> that all sounds a bit complicated
10 2011-12-24T00:37:08 <ThomasWaldmann> isn't there a more simple way?
11 2011-12-24T00:49:45 <izibi> you need some way to pass that information
12 2011-12-24T00:50:03 <izibi> and using a standard http redirect isn't possible as far as i know
13 2011-12-24T00:59:46 <ThomasWaldmann> i didn't look at the stuff yet, so i can't comment
14 2011-12-24T01:00:07 <ThomasWaldmann> but it is quite a standard problem, so maybe do some research
15 2011-12-24T01:04:52 <izibi> what? only 10h left? when did i claim tasks at noon? :D
16 2011-12-24T01:04:59 <izibi> can you please extend the deadline? ;)
17 2011-12-24T01:05:17 *** raignarok
18 2011-12-24T01:11:00 <ThomasWaldmann> can we have a simple rule for the future?
19 2011-12-24T01:11:47 <ThomasWaldmann> like "never request review/task state change/whatever" without giving a url?
20 2011-12-24T01:14:20 <izibi> i guess that's too difficult :P
21 2011-12-24T01:14:24 <izibi> http://www.google-melange.com/gci/task/view/google/gci2011/7170218
22 2011-12-24T01:15:54 <izibi> thx
23 2011-12-24T01:16:26 <ThomasWaldmann> btw, iirc werkzeug has some "is ajax" attribute
24 2011-12-24T01:17:17 <izibi> let's see, i only looked at the flask docs
25 2011-12-24T01:19:42 <izibi> nope, can't find something
26 2011-12-24T01:20:03 <izibi> maybe you mix it up with django? they have it
27 2011-12-24T01:27:26 <ThomasWaldmann> i don't use django
28 2011-12-24T01:27:46 <ThomasWaldmann> review done
29 2011-12-24T01:28:00 <ThomasWaldmann> TheSheep: can you review that? ^^
30 2011-12-24T01:28:50 <ThomasWaldmann> is_xhr
31 2011-12-24T01:29:00 <ThomasWaldmann> izibi: ^^
32 2011-12-24T01:29:07 <izibi> ThomasWaldmann: "why do you need macros if it is just one call?" <-- ?
33 2011-12-24T01:29:46 <izibi> you won't find that if you search for "ajax"... :D
34 2011-12-24T02:02:38 <izibi> gn
35 2011-12-24T02:20:51 *** raignarok
36 2011-12-24T02:52:11 *** bilal
37 2011-12-24T03:15:42 <bilal> ThomasWaldmann: Hi, I've added the comment to the task on melange
38 2011-12-24T03:15:56 * bilal is cdbs btw
39 2011-12-24T03:24:00 *** bilal
40 2011-12-24T03:25:57 *** MattMaker
41 2011-12-24T03:32:28 *** qxcv
42 2011-12-24T04:50:03 *** bilal
43 2011-12-24T05:10:19 *** MattMaker
44 2011-12-24T05:14:36 *** bilal
45 2011-12-24T08:06:29 *** qxcv
46 2011-12-24T08:19:21 *** qxcv
47 2011-12-24T11:20:05 *** raignarok
48 2011-12-24T11:40:10 *** raignarok
49 2011-12-24T11:56:25 *** greg_f
50 2011-12-24T12:43:37 <eSyr> ThomasWaldmann: http://www.google-melange.com/gci/task/view/google/gci2011/7121279 hm.
51 2011-12-24T13:36:52 *** raignarok
52 2011-12-24T13:47:48 *** raignarok
53 2011-12-24T14:19:16 *** raignarok
54 2011-12-24T14:20:00 *** qxcv
55 2011-12-24T15:52:53 *** bilal
56 2011-12-24T16:06:55 <ThomasWaldmann> moin
57 2011-12-24T16:07:43 <ThomasWaldmann> eSyr: maybe google should add some "unclaim reason" input field
58 2011-12-24T16:23:41 <ThomasWaldmann> izibi: wouldn't it be easier to just render all the usersettings html, subdivide it into some divs for the different sections and then just show the right diff for the current tab and hide everything else?
59 2011-12-24T16:24:24 <ThomasWaldmann> did you research how other software/sites do such stuff?
60 2011-12-24T16:26:41 <ThomasWaldmann> s/diff/div/
61 2011-12-24T16:30:09 <izibi> ThomasWaldmann: why do you want to generate html that doesn't even gets displayed at all?
62 2011-12-24T16:30:15 <izibi> wasted resources
63 2011-12-24T16:33:12 <bilal> ThomasWaldmann: here, I've answered your question on the melange task: http://www.google-melange.com/gci/task/view/google/gci2011/7176230
64 2011-12-24T16:34:20 *** greg_f
65 2011-12-24T16:34:52 <izibi> ThomasWaldmann: yes i am. who had the idea to use spaces in the first place. :P
66 2011-12-24T16:40:57 <ThomasWaldmann> it should be 4 spaces per indentation, as in python.
67 2011-12-24T16:41:26 <ThomasWaldmann> or at least the style you found there (which seems to be 8 spaces)
68 2011-12-24T16:41:39 <ThomasWaldmann> but mixing spaces and tabs is disallowed.
69 2011-12-24T16:42:46 <ThomasWaldmann> izibi: it is very likely that someone who visits userprefs does not only look at one of the pages, but at multiple ones.
70 2011-12-24T16:43:49 <ThomasWaldmann> also, it is questionable whether using lots of code, doing multiple ajax requests, etc. is cheaper in the end than just rendering a few input elements more.
71 2011-12-24T16:47:54 <ThomasWaldmann> for the user i guess it would mean a little more initial latency (maybe not noticeable), but immediate response when switching between tabs.
72 2011-12-24T16:51:00 <ThomasWaldmann> bilal: i am looking at that right now
73 2011-12-24T16:51:03 *** raignarok
74 2011-12-24T16:54:57 <ThomasWaldmann> bilal: you are talking about a "flask change", can you be more specific?
75 2011-12-24T16:55:28 <bilal> ThomasWaldmann: I meant, flask now automatically urlencodes the url that is returned by url_for
76 2011-12-24T16:56:06 <bilal> this didn't happen earlier
77 2011-12-24T16:56:13 <bilal> due to either a configuration change with moin
78 2011-12-24T16:56:22 <bilal> or a change with a newer flask version
79 2011-12-24T16:56:22 <ThomasWaldmann> so, did you find why +diff is +diff and +diffraw is %2bdiffraw?
80 2011-12-24T16:56:38 <bilal> +diff is also %2Bdiff
81 2011-12-24T16:57:32 <ThomasWaldmann> http://test.moinmo.in/+history/Home look at the html source
82 2011-12-24T16:57:55 <ThomasWaldmann> usually it is /+xxxx there not /%2Bxxxxx
83 2011-12-24T16:58:46 <bilal> hmm
84 2011-12-24T16:58:50 * bilal digs deeper
85 2011-12-24T17:01:48 <bilal> ThomasWaldmann: because urlencoding is done only for url_for
86 2011-12-24T17:02:38 <ThomasWaldmann> well, i'ld suspect we use url_for everywhere
87 2011-12-24T17:03:14 <bilal> this is tricky then..
88 2011-12-24T17:05:20 <bilal> ThomasWaldmann: wait
89 2011-12-24T17:05:54 <bilal> ThomasWaldmann: http://pastebin.com/Dbtu3DwE
90 2011-12-24T17:06:03 <bilal> see, many GET requests in fact use %2B
91 2011-12-24T17:06:09 <bilal> even diff is %2B
92 2011-12-24T17:07:38 <bilal> ThomasWaldmann: okay, on my system when I wget the source code of the history page, most links with + actually have %2B in its place
93 2011-12-24T17:07:50 <bilal> but that doesn't work with test.moinmo.in
94 2011-12-24T17:07:59 <ThomasWaldmann> huh?
95 2011-12-24T17:08:14 * ThomasWaldmann tries
96 2011-12-24T17:12:12 <ThomasWaldmann> i just wgetted $ wget http://127.0.0.1:8080/+history/Home
97 2011-12-24T17:12:24 <ThomasWaldmann> not a single %2B in there
98 2011-12-24T17:12:57 <bilal> for me, I have like tons of those
99 2011-12-24T17:13:13 <bilal> ThomasWaldmann: can you update your flask and see if the issue arises then?
100 2011-12-24T17:13:48 <bilal> the same page, I have many %2Bs
101 2011-12-24T17:15:19 <ThomasWaldmann> what flask do you have?
102 2011-12-24T17:15:46 <ThomasWaldmann> Flask==0.8 here
103 2011-12-24T17:17:11 <ThomasWaldmann> Werkzeug==0.8.1
104 2011-12-24T17:18:37 <bilal> one sec
105 2011-12-24T17:19:16 <ThomasWaldmann> looks like it is a werkzeug 0.8.1 -> 0.8.2 change
106 2011-12-24T17:19:33 <bilal> I have flask 0.8, Werkzeug 0.8.2
107 2011-12-24T17:21:45 <ThomasWaldmann> "Fixed a few unicode issues with non-ascii characters being hardcoded in URL rules." form werkzeug 0.8.2 changelog
108 2011-12-24T17:24:05 <ThomasWaldmann> https://github.com/mitsuhiko/werkzeug/commit/67c8b8747ee638c963ef311e2fb733ebe7dd390a
109 2011-12-24T17:24:33 <ThomasWaldmann> ok, so the differences we have seen was due to that
110 2011-12-24T17:27:10 <bilal> makes sense
111 2011-12-24T17:27:18 <bilal> ThomasWaldmann: so
112 2011-12-24T17:27:25 <ThomasWaldmann> bilal: just asked mitsuhiko about whether not having + there is intended
113 2011-12-24T17:27:35 <bilal> okie
114 2011-12-24T17:28:07 <ThomasWaldmann> well, as we are having now different behaviour for different werkzeug versions, i think we must accept BOTH for now
115 2011-12-24T17:30:41 <bilal> hmm
116 2011-12-24T17:31:27 <bilal> so, should I add a try/catch there, that checks if AssertError or something is thrown, and then runs the second test with %2B, and if that also fails, then it throws the exception?
117 2011-12-24T17:31:35 <bilal> brb
118 2011-12-24T17:32:51 <ThomasWaldmann> assert result in [expected1, expected2, ]
119 2011-12-24T17:33:39 <ThomasWaldmann> and a comment about <= 0.8.1 and 0.8.2 would be good, so we can simplify this again in the future after that is stable
120 2011-12-24T17:58:15 <bilal> okay
121 2011-12-24T18:00:29 <izibi> ThomasWaldmann: only saving the form does an ajax request
122 2011-12-24T18:00:59 <izibi> ThomasWaldmann: the first get requests loads all the tabs, but if a user saves something, that does an ajax request
123 2011-12-24T18:04:34 * ThomasWaldmann must go, bbl
124 2011-12-24T18:08:37 <bilal> exit
125 2011-12-24T18:08:41 <bilal> err whoops
126 2011-12-24T18:08:42 *** bilal
127 2011-12-24T18:55:27 *** raignarok
128 2011-12-24T19:05:19 *** raignarok
129 2011-12-24T21:04:36 <dreimark> moin
130 2011-12-24T21:35:46 *** pkumar
131 2011-12-24T21:58:26 *** pkumar
132
