Page MenuHomePhabricator

elm_spinner: Fixed to apply the %d format.
ClosedPublic

Authored by CHAN on Feb 19 2020, 3:43 AM.

Details

Summary

The part object does not apply the logic in efl_ui_format, so it does not work correctly when you format it with %d.

This is the commit that gets the necessary part of the logic of efl_ui_format.

Test Plan

elementary_test

Diff Detail

Repository
rEFL core/efl
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
CHAN created this revision.Feb 19 2020, 3:43 AM

It seems that this patch has no reviewers specified. If you are unsure who can review your patch, please check this wiki page and see if anyone can be added: https://phab.enlightenment.org/w/maintainers_reviewers/

CHAN requested review of this revision.Feb 19 2020, 3:43 AM
CHAN updated this revision to Diff 29122.Feb 19 2020, 3:48 AM

Code clean up.

CHAN edited the summary of this revision. (Show Details)Feb 19 2020, 3:50 AM
CHAN edited the test plan for this revision. (Show Details)
CHAN added a reviewer: Jaehyun_Cho.
Jaehyun_Cho accepted this revision.Feb 19 2020, 4:08 AM
This revision is now accepted and ready to land.Feb 19 2020, 4:08 AM
Closed by commit rEFLb8a24679a6f7: elm_spinner: Fixed to apply the %d format. (authored by Woochanlee <wc0917.lee@samsung.com>, committed by Jaehyun_Cho). · Explain WhyFeb 19 2020, 4:10 AM
This revision was automatically updated to reflect the committed changes.

This amount of duplicated code worries me.

What is exactly the problem that this is fixing? What should I look for in elementary_test?
Slider's indicator seems to work fine, for ints and doubles.

Additionally, there is the following:

  • this code tries to model 3 different states with a 1 bit var. We have the situation of int,float & nothing. So in a usecase where you do not set any % at all in the format string, we will append the int anyways, where i am not sure what the behaviour of the strbuf is.
  • indi_format_int looks like a totally bad variable name
  • This is straight away copied from efl_ui_format.c, however, simply removing like 1477 has the code beeing executed that was copied here.
  • Switch statement has a default case, however all cases that are possible are covered.
  • Commit messages claims this is for slider, but it was written in spinner.

Sorry, I made D11394 to revert this.
I will discuss this patch with @CHAN again.