Page MenuHomePhabricator

UI: Add layout for filter window.
ClosedPublic

Authored by Deepwarrior on Aug 29 2017, 5:34 AM.

Details

Reviewers
NikaWhite
i.furs
Summary

Add layout for genlists with rules and with names.

Add function for config directory.

Use XDG_CONFIG_HOME, if it isn`t exist, take ~/.config/Efl_profiling_viewer

Split filters from UI to other module.

Add basic callbacks for creating new rules and events.
Add saving rule into file.
Add function for apply rules.

Filters: replace label with entry. Save all filters in eet file.

Use full widget style instead default for names, that can be changed: rule names
and events in rules. Add callbacks on changing this entries for changing name
in struct.
Save all rules in eet file when module shutdowns.

UI: fix dead initialization.

@fix

Fix open description page in browser.

Replace buffer size with correct value.
@fix

Add events to rule by clicking on button in group genlist item.

Check if rule active when existing rules filling in table.

Test Plan

Load log. Open filter window. Add new Rule, add new events
to this rule. Add second rule with events that contradict to events
in first rule. Check how rules work sequently. Reopen filter window.
All currently active rules must be turned on. Reopen profiler.
All rules must be same as in previous launch.

Diff Detail

Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 4386
Build 4451: arc lint + arc unit
Deepwarrior created this revision.Aug 29 2017, 5:34 AM
NikaWhite requested changes to this revision.Aug 29 2017, 6:16 AM

Huge amount of warning during compilation:

/home/nikawhite/repos/enlightenment/efl_profiler_viewer/src/lib/filters.c: In function '_thread_event_label':
/home/nikawhite/repos/enlightenment/efl_profiler_viewer/src/lib/filters.c:92:27: warning: unused parameter 'data' [-Wunused-parameter]
 _thread_event_label(void *data, Evas_Object *obj EINA_UNUSED, const char *part EINA_UNUSED)
                           ^~~~
/home/nikawhite/repos/enlightenment/efl_profiler_viewer/src/lib/filters.c: In function '_state_event_label':
/home/nikawhite/repos/enlightenment/efl_profiler_viewer/src/lib/filters.c:98:26: warning: unused parameter 'data' [-Wunused-parameter]
 _state_event_label(void *data, Evas_Object *obj EINA_UNUSED, const char *part EINA_UNUSED)
                          ^~~~
/home/nikawhite/repos/enlightenment/efl_profiler_viewer/src/lib/filters.c: In function '_event_entry_changed_cb':
/home/nikawhite/repos/enlightenment/efl_profiler_viewer/src/lib/filters.c:131:61: warning: unused parameter 'event_info' [-Wunused-parameter]
 _event_entry_changed_cb(void *data, Evas_Object *obj, void *event_info)
                                                             ^~~~~~~~~~
/home/nikawhite/repos/enlightenment/efl_profiler_viewer/src/lib/filters.c: In function 'event_filtred_changed_cb':
/home/nikawhite/repos/enlightenment/efl_profiler_viewer/src/lib/filters.c:139:62: warning: unused parameter 'event_info' [-Wunused-parameter]
 event_filtred_changed_cb(void *data, Evas_Object *obj, void *event_info)
                                                              ^~~~~~~~~~

I think we are need to enable -Werror compilation flag for library

Choose single style for static function notation and use it.

.gitignore
19

build/

src/lib/filters.c
20

static?

30

not static?
Really non-informative variable name. Please update this.

178

You forget delete something

192

what about:

elm_check_state_set(check, !thd->cpuuse_filtered);

This could be apply aroud your code.

282

why not?

452

Move declaration to the beginning of function code

557

Delete

619

leak: conf_dir and rule_path should be freed here.
Same inside save_rules

src/lib/ui.c
2532

100% that event_info not NULL?

2552

Global_Data --> GLOBAL_DATA

2576

get,In_Thread_Data really bad name for signal. in,thread,data,request, or something that respond for this action more accurate.

This revision now requires changes to proceed.Aug 29 2017, 6:16 AM

I`ll commit other changes soon.

src/lib/filters.c
178

Here must be function for deleting elements, it will appear in next commit.

282

I had errors after loading rule names from eet file.
It is temporary solution.

I find wrong behaviour.
Reproduce:

  1. Launch Viewer with log file
  2. Open filters window
  3. Add new rule "New_Rule" with turned off "animator" event inside state_events group
  4. Enable "New_Rule"
  5. Check that filters works fine
  6. Open another log file

Result: filter rule doesn't applyed. But in window rule displayed as enabled.
Expected: filter rule applyed.

Thank you for review, I have fixed your comments.

NikaWhite requested changes to this revision.Sep 1 2017, 2:13 AM
NikaWhite added inline comments.
src/lib/filters.c
133

need to check on NULL before free.
Here is the memory leak.

166

Very strange thing.
Firstly, please describe why this need.
Secondly, need to find way how to avoid this.

241

This line and code until end of function should be reused. My suggestion move that code into internal help function

309

need to free here

This revision now requires changes to proceed.Sep 1 2017, 2:13 AM

Please rebase this one

Deepwarrior marked 12 inline comments as done.Sep 7 2017, 2:33 AM

Please rebase this one

$ git pull --rebase origin master
From https://git.enlightenment.org/devs/nikawhite/efl_profiler_viewer

  • branch master -> FETCH_HEAD

Current branch arcpatch-D5139 is up to date.

What about:

  1. git pull origin master
  2. arc patch --nobranch D5139

?

NikaWhite added inline comments.Sep 7 2017, 4:30 AM
src/lib/filters.c
187

Need to add elm_entry_single_line_set(entry, EINA_TRUE);

334

Need to add: elm_entry_single_line_set(entry, EINA_TRUE);

Deepwarrior updated this revision to Diff 12205.Sep 7 2017, 4:30 AM

Fix memory leak.

Deepwarrior marked 4 inline comments as done.Sep 7 2017, 4:41 AM

What about:

  1. git pull origin master
  2. arc patch --nobranch D5139 ?

Same:

/efl_profiling_tool/build$ git pull origin master
From https://git.enlightenment.org/devs/nikawhite/efl_profiler_viewer

  • branch master -> FETCH_HEAD

Already up-to-date.

Deepwarrior updated this revision to Diff 12206.Sep 7 2017, 4:50 AM

Rebase and add singleline for entry.

NikaWhite requested changes to this revision.Sep 7 2017, 5:30 AM

I am so much dislike arcanist... anyway you should finish this rebase well.

This revision now requires changes to proceed.Sep 7 2017, 5:30 AM
Deepwarrior updated this revision to Diff 12207.Sep 7 2017, 7:56 AM

Apply all changes to first diff.

Deepwarrior updated this revision to Diff 12208.Sep 7 2017, 8:06 AM

I think, it is final fix.

NikaWhite accepted this revision.Sep 8 2017, 1:25 AM

Good job!

This revision is now accepted and ready to land.Sep 8 2017, 1:25 AM
NikaWhite closed this revision.Sep 8 2017, 1:25 AM