On Mon, 28 Mar 2011 12:04:19 +0900 cnook <kimci...@gmail.com> said:
too many problems with this patch :( can you fix the below and re-submit?
1. fails. malformed patch even.
patching file src/lib/elm_diskselector.c
Hunk #5 FAILED at 136.
Hunk #19 succeeded at 780 with fuzz 2.
1 out of 29 hunks FAILED -- saving rejects to file
src/lib/elm_diskselector.c.rej patching file src/lib/Elementary.h.in
patching file src/bin/test_diskselector.c
patching file data/themes/default.edc
patch: **** malformed patch at line 553:
2. math relying on operator precedence rather than being explicit with ()'s:
+ evas_object_resize(wd->main_box, w / wd->display_item_num *
(wd->item_count + CEIL(wd->display_item_num) * 2), h);
+ evas_object_resize(wd->main_box, w / wd->display_item_num *
(wd->item_count + CEIL(wd->display_item_num)), h);
3. divide by 0 waiting to happen:
+ (int)(w / wd->display_item_num), 0);
(you never check the return of atoi so it can happily return 0 or theme could
literally use a 0)
4. no space between if and ():
+ if(!wd->display_item_num_by_api)
5. no check of atoi result to stop invalid values (<= 0):
+ if (str) wd->display_item_num = atoi(str);
6. no check of atoi() to check for <= -2:
+ if (str) wd->minw = atoi(str);
+ if (str) wd->minh = atoi(str);
7. no check of atoi result to stop invalid values:
+ if (str) wd->display_item_num = atoi(str);
8. use: if (!strcmp(...)):
+ if (strcmp(item->label, it->label) == 0) edje_object_signal_emit
(item->base.view, "elm,state,selected", "elm");
9. add space between void and * and no space between ) and it->:
+ if (it->func) it->func((void*) it->base.data, it->base.widget, it);
10. more operator precedence reliance here. use ()'s to collect statements:
+ if (x > w / wd->display_item_num * (wd->item_count +
(wd->display_item_num % 2)))
+ x - w / wd->display_item_num *
wd->item_count,
+ x + w / wd->display_item_num *
wd->item_count,
11. add space between for and (:
+ for(i = 2; i < CEIL(wd->display_item_num); i++)
+ for(i = 3; i <= CEIL(wd->display_item_num); i++)
+ for(i = 2; i < CEIL(wd->display_item_num); i++)
+ for(i = 3; i <= CEIL(wd->display_item_num); i++)
+ for(idx = 0; idx < sizeof(month_list) / sizeof(month_list[0]); idx++)
+ for(idx = 1; idx < 31; idx++)
12. please declare vars at top of function or {} section where you can:
+ char *month_list[] = {
+ char date[3];
> Hi All,
>
> Thanks for your response and suggestion always.
> I have attached new patch file which has following modification.
>
> - remove warning message
> - support setting by theme and api
>
> Others are also reasonable suggestion, some of them especially vertical mode
> would be fine. But till now there is no design for that as raster mentioned.
> setting min/max "display item num" is reasonable also but user can set
> content size differently. So if elm_diskselector restrict its min/max
> "display item num", it would occur that user cannot set "display item num"
> more than max value even though there is enough space.
>
> Thanks again.
>
> Sincerely,
> Shinwoo Kim.
>
> 2011/3/25 Carsten Haitzler <ras...@rasterman.com>
>
> > On Wed, 23 Mar 2011 08:19:13 -0700 Daniel Juyung Seo <seojuyu...@gmail.com
> > >
> > said:
> >
> > you're going to hate me :)
> >
> > you did go the right way - theme defines default, code can override...
> > BUT..
> > you are ALSO missing a fit POLICY. you are fitting just N items in the
> > visible
> > region. i think... you are missing the ability to say "fit as many items
> > ads
> > you can" either based on largest item in list (homogeneous layout in the
> > box)
> > or have all items a different size (actually i might argue that this is
> > dubiously useful except maybe in a case where MOST items are small and 1 or
> > 2
> > are longer eg a list like: 1,2,3,4,5,NONE).
> >
> > so THEME should provide the default FIT policy (fixed count, best fit,
> > compact
> > fit). fixed count uses the theme number defined as you did already and code
> > can
> > override that, and other policies as above. also code should be allowed to
> > override this too.
> >
> > (and yes a horizontal scrolling list of items where they change size
> > horizontally is not that great as gustavo mentioned... but i know that you
> > didn't design the widget, so not a lot of use going on about that - BUT his
> > point implies that we should have a vertical diskselector mode too... use
> > that
> > for horizontally expanding items, and vice-versa).
> >
> > > I attached a screen shot :)
> > > This is a screen shot from elementary_test "Disk Selector" which cnook
> > > attached.
> > >
> > > Btw, cnook, I have one comment.
> > > _item_click_cb()'s parameter is wrong.
> > > Please check elm_diskselector.c code.
> > >
> > > elm_diskselector.c: In function ‘_item_new’:
> > > elm_diskselector.c:137: warning: passing argument 4 of
> > > ‘edje_object_signal_callback_add’ f
> > > rom incompatible pointer type
> > > /usr/local/include/edje-1/Edje.h:550: note: expected ‘Edje_Signal_Cb’
> > > but argument is of t
> > > ype ‘void (*)(void *, struct Evas_Object *, void *)’
> > >
> > > Thanks.
> > > Daniel Juyung Seo (SeoZ)
> > >
> > >
> > >
> > >
> > > On Wed, Mar 23, 2011 at 7:15 AM, Gustavo Sverzut Barbieri
> > > <barbi...@profusion.mobi> wrote:
> > > > On Wed, Mar 23, 2011 at 1:54 PM, cnook <kimci...@gmail.com> wrote:
> > > >> Dear All,
> > > >>
> > > >> Thanks for your response.
> > > >>
> > > >> I have attached "elm_diskselector" patch using API and getting default
> > > >> value from theme.
> > > >> But the default value from "elementary/themes/default.edc" is applied
> > only.
> > > >> If user wants new default value from his/her own theme file, it will
> > not be
> > > >> applied.
> > > >>
> > > >>
> > > >> Why I have changed like this.. because there is one case I worry
> > about
> > > >
> > > >
> > > > Do you have the screenshot? Nobody replied to my mail.
> > > >
> > > >
> > > > --
> > > > Gustavo Sverzut Barbieri
> > > > http://profusion.mobi embedded systems
> > > > --------------------------------------
> > > > MSN: barbi...@gmail.com
> > > > Skype: gsbarbieri
> > > > Mobile: +55 (19) 9225-2202
> > > >
> > > >
> > ------------------------------------------------------------------------------
> > > > Enable your software for Intel(R) Active Management Technology to meet
> > the
> > > > growing manageability and security demands of your customers.
> > Businesses
> > > > are taking advantage of Intel(R) vPro (TM) technology - will your
> > software
> > > > be a part of the solution? Download the Intel(R) Manageability Checker
> > > > today! http://p.sf.net/sfu/intel-dev2devmar
> > > > _______________________________________________
> > > > enlightenment-devel mailing list
> > > > enlightenment-devel@lists.sourceforge.net
> > > > https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
> > > >
> >
> >
> > --
> > ------------- Codito, ergo sum - "I code, therefore I am" --------------
> > The Rasterman (Carsten Haitzler) ras...@rasterman.com
> >
> >
--
------------- Codito, ergo sum - "I code, therefore I am" --------------
The Rasterman (Carsten Haitzler) ras...@rasterman.com
------------------------------------------------------------------------------
Create and publish websites with WebMatrix
Use the most popular FREE web apps or write code yourself;
WebMatrix provides all the features you need to develop and
publish your website. http://p.sf.net/sfu/ms-webmatrix-sf
_______________________________________________
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
