On Thu, Jan 7, 2016 at 3:56 AM, Jean-Philippe André <[email protected]> wrote: > On 7 January 2016 at 03:38, Cedric BAIL <[email protected]> wrote: >> >> > [...] >> > Edje_Common.h >> >> edje_size_class_list ( ) >> > > I'm not a fan of this. It's just like text_class and color_class, so fine, > but I think an iterator would have been better. > Consistency is important, though.
Yes, for consistency alone I agree on this API (Even if I prefer iterator). >> > edje_mmap_size_class_iterator_new ( Eina_File* f ) >> > edje_mmap_text_class_iterator_new ( Eina_File* f ) >> > edje_size_class_active_iterator_new ( ) >> > edje_text_class_active_iterator_new ( ) > > OK, so those APIs are in fact useless right now. > > They return an iterator to a private struct: Edje_Size_Class or > Edje_Text_Class. Oh, interesting I didn't notice we didn't export those structure. We do export Edje_Color_Class and I was expecting the same behavior for this two class. > A few solutions: > - Make those structs public and STABLE Any reason why not doing so ? > - Keep them opaque but add APIs to inspect them (eg. > edje_size_class_iterator_item_name_get() , > edje_size_class_iterator_item_del() ...) I don't like adding accessor that will need binding later on. > - Remove these APIs (they return a different set than the class_list > functions) This API has been useful for color class in the past, I am guessing it would be the same for this too. > Also, what about EO and bindings here? I am guessing we could expose a root singleton Edje object that this function get attached to, but the main question is would that be useful to an application ? I am not sure as this is something that elementary do act on. No obvious answer at this stage. > What do you guys think? > > All those APIs need test cases btw. True. >> > Edje_Legacy.h >> > edje_object_size_class_del ( Evas_Object* obj, char const* size_class ) >> > edje_object_text_class_del ( Evas_Object* obj, char const* text_class ) >> >> I am fine with all the above one. > > Are those necessary? Adding legacy APIs for new things. > No EO equivalent? Oh, that's a mistake it should have been an Eo API with automatically generated code for legacy. I didn't notice that. Thanks for pointing it, will fix it right away. It actually also apply to color_class_del ! >> eina_value.h >> > eina_value_optional_empty_is ( Eina_Value const* value, Eina_Bool* >> > is_empty ) >> > eina_value_optional_empty_new ( ) >> > eina_value_optional_type_get ( Eina_Value* value )> >> eina_value_optional_new ( Eina_Value_Type const* subtype, void const* >> > value ) >> > eina_value_optional_pget ( Eina_Value* value, void* subvalue ) >> > eina_value_optional_pset ( Eina_Value* value, Eina_Value_Type const* >> > subtype, void const* subvalue ) >> > eina_value_optional_reset ( Eina_Value* value ) >> > EINA_VALUE_TYPE_OPTIONAL [data] >> > > So those optional values are really just values that may be unset? > I'm not sure why a normal eina_value can't just have the empty property, > and have a void type that stores nothing. > Did I miss something? I think that Felipe can explain that better than I can. >> > eina_matrix.h >> > eina_matrix2_array_set ( Eina_Matrix2* m, double const* v ) >> > eina_matrix2_copy ( Eina_Matrix2* dst, Eina_Matrix2 const* src ) >> > eina_matrix2_identity ( Eina_Matrix2* m ) >> > eina_matrix2_inverse ( Eina_Matrix2* out, Eina_Matrix2 const* mat ) >> > eina_matrix2_multiply ( Eina_Matrix2* out, Eina_Matrix2 const* mat_a, >> > Eina_Matrix2 const* mat_b ) >> > eina_matrix2_multiply_copy ( Eina_Matrix2* out, Eina_Matrix2 const* >> > mat_a, Eina_Matrix2 const* mat_b ) >> > eina_matrix2_type_get ( Eina_Matrix2 const* m ) >> > eina_matrix2_values_get ( Eina_Matrix2 const* m, double* xx, double* xy, >> > double* yx, double* yy ) >> > eina_matrix2_values_set ( Eina_Matrix2* m, double xx, double xy, double >> > yx, double yy ) >> > eina_matrix3_array_set ( Eina_Matrix3* m, double const* v ) >> > eina_matrix3_copy ( Eina_Matrix3* dst, Eina_Matrix3 const* src ) >> > eina_matrix3_multiply ( Eina_Matrix3* out, Eina_Matrix3 const* mat_a, >> > Eina_Matrix3 const* mat_b ) >> > eina_matrix3_multiply_copy ( Eina_Matrix3* out, Eina_Matrix3 const* >> > mat_a, Eina_Matrix3 const* mat_b ) >> > eina_matrix3_position_transform_set ( Eina_Matrix3* out, double const >> > p_x, double const p_y ) >> > eina_matrix3_scale_transform_set ( Eina_Matrix3* out, double s_x, double >> > s_y ) >> > eina_matrix4_array_set ( Eina_Matrix4* m, double const* v ) >> > eina_matrix4_copy ( Eina_Matrix4* dst, Eina_Matrix4 const* src ) >> > eina_matrix4_multiply_copy ( Eina_Matrix4* out, Eina_Matrix4 const* >> > mat_a, Eina_Matrix4 const* mat_b ) >> > eina_matrix4_ortho_set ( Eina_Matrix4* m, double left, double right, >> > double bottom, double top, double dnear, double dfar ) >> > eina_normal3_matrix_get ( Eina_Matrix3* out, Eina_Matrix4 const* m ) >> > >> > eina_quaternion.h >> > eina_quaternion_angle_plains ( Eina_Quaternion* a, Eina_Quaternion* b ) >> > eina_quaternion_array_set ( Eina_Quaternion* dst, double const* v ) >> > eina_quaternion_copy ( Eina_Quaternion* dst, Eina_Quaternion const* src ) >> > eina_quaternion_distance_get ( Eina_Quaternion const* a, Eina_Quaternion >> > const* b ) >> > eina_quaternion_distance_square_get ( Eina_Quaternion const* a, >> > Eina_Quaternion const* b ) >> > eina_quaternion_homogeneous_regulate ( Eina_Quaternion* out, >> > Eina_Quaternion const* v ) >> > eina_quaternion_inverse ( Eina_Quaternion* out, Eina_Quaternion const* q >> ) >> > eina_quaternion_length_get ( Eina_Quaternion const* v ) >> > eina_quaternion_length_square_get ( Eina_Quaternion const* v ) >> > eina_quaternion_subtract ( Eina_Quaternion* out, Eina_Quaternion const* >> > a, Eina_Quaternion const* b ) >> > eina_quaternion_transform ( Eina_Quaternion* out, Eina_Quaternion const* >> > v, Eina_Matrix4 const* m ) >> >> I am fine with all the above eina change. >> > > Do they need to be EAPI? Why not static inline? > (I know some of them are quite big, though) Well, some of them are indeed big and I just didn't really bother picking the one that could be inline from the one that shouldn't. My logic was that this could be done at a later state and the new inline function will get a copy in eina_abi.c >> > eina_str.h >> > eina_str_base64_decode ( char const* src, int* decoded_str_len ) >> > eina_str_base64_encode ( unsigned char const* src, unsigned int len ) >> > eina_str_base64url_decode ( char const* src, int* decoded_str_len ) >> > eina_str_base64url_encode ( unsigned char const* src, unsigned int len ) >> > eina_strftime ( char const* format, struct tm const* tm ) >> >> I have been wondering for some time if we shouldn't move this base64 >> function in their own header/namespace. It would be easier to find ? >> Any though about that ? >> > > Why not. > Or maybe in emile? :) Oh, good point. Emile it should be. >> > eina_strbuf.h >> > eina_strbuf_substr_get ( Eina_Strbuf* buf, size_t pos, size_t len ) >> > eina_strbuf_tolower ( Eina_Strbuf* buf ) >> > >> > eina_tmpstr.h >> > eina_tmpstr_manage_new ( char* str ) >> > eina_tmpstr_manage_new_length ( char* str, size_t length ) >> > >> > eina_unicode.h >> > eina_unicode_unicode_to_utf8_range ( Eina_Unicode const* uni, int ulen, >> > int* _len ) >> > >> > >> > eina_vector.h >> > eina_vector2_add ( Eina_Vector2* out, Eina_Vector2 const* a, >> > Eina_Vector2 const* b ) >> > eina_vector2_array_set ( Eina_Vector2* dst, double const* v ) >> > eina_vector2_copy ( Eina_Vector2* dst, Eina_Vector2 const* src ) >> > eina_vector2_distance_get ( Eina_Vector2 const* a, Eina_Vector2 const* b >> ) >> > eina_vector2_distance_square_get ( Eina_Vector2 const* a, Eina_Vector2 >> > const* b ) >> > eina_vector2_dot_product ( Eina_Vector2 const* a, Eina_Vector2 const* b ) >> > eina_vector2_homogeneous_direction_transform ( Eina_Vector2* out, >> > Eina_Matrix3 const* m, Eina_Vector2 const* v ) >> > eina_vector2_homogeneous_position_transform ( Eina_Vector2* out, >> > Eina_Matrix3 const* m, Eina_Vector2 const* v ) >> > eina_vector2_length_get ( Eina_Vector2 const* v ) >> > eina_vector2_length_square_get ( Eina_Vector2 const* v ) >> > eina_vector2_negate ( Eina_Vector2* out, Eina_Vector2 const* v ) >> > eina_vector2_normalize ( Eina_Vector2* out, Eina_Vector2 const* v ) >> > eina_vector2_scale ( Eina_Vector2* out, Eina_Vector2 const* v, double >> > scale ) >> > eina_vector2_set ( Eina_Vector2* dst, double x, double y ) >> > eina_vector2_subtract ( Eina_Vector2* out, Eina_Vector2 const* a, >> > Eina_Vector2 const* b ) >> > eina_vector2_transform ( Eina_Vector2* out, Eina_Matrix2 const* m, >> > Eina_Vector2 const* v ) >> >> Fine with all the above eina function. >> > > They need test cases. Indeed. That was one of the reason to expose them. We have 2 weeks to get that. -- Cedric BAIL ------------------------------------------------------------------------------ _______________________________________________ enlightenment-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
