Enhanced version of the Filter gramplet #1721

kkujansuu · 2024-05-11T17:58:45Z

This is an enhanced version of the Filter gramplet. The enhancements are

The elapsed time is displayed.
A new 'Define filter' button that opens a filter editor dialog and automatically populates it with the rules that correspond to values given in the gramplet.
The person filter has two new fields: Birth place and Death place
The default confidence value in the Citation filter is 'Very low'.
Place filter: An error message is displayed if the user fills in the 'Within' value but no place is active

emyoulation · 2024-05-11T18:07:30Z

Very nice!

You have one of the few functional aborts in Gramps for your Filter Params test feature.

I cannot count the number of times that the Filter progress dialog has appeared and I have wished for an abort. (Usually because I had set a parameter incorrectly.)

Could that test abort routine be made compatible with the Filter Progress dialog?

It would probably have to abort to "unfiltered"... would that be intuitive enough?

kkujansuu · 2024-05-11T19:06:43Z

Could that test abort routine be made compatible with the Filter Progress dialog?

Not easily. la 11. toukok. 2024 klo 21.07 Emyoulation or BAMaustin ( ***@***.***) kirjoitti:

…

Very nice! You have one of the few functional aborts in Gramps for your Filter Params test feature. I cannot count the number of times that the Filter progress dialog has appeared and I have wished for an abort. (Usually because I had set a parameter incorrectly.) Could that test abort routine be made compatible with the Filter Progress dialog? It would probably have to abort to "unfiltered"... would that intuitive enough? I — Reply to this email directly, view it on GitHub <#1721 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AEIYOXTFFGNKELT2TSWX74TZBZM7RAVCNFSM6AAAAABHSDFO3WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMBVHE3TQNJQHE> . You are receiving this because you authored the thread.Message ID: ***@***.***>

emyoulation · 2024-05-11T19:21:44Z

Not easily.

Understood and thanks for looking... and another 'thanks' for not being aggravated by the extra ask.

Looks like the PR test fails are formatting compliance. I ran into similar fails for the last PR that I tried. And they would have had the same fails for the module before my changes.

Maybe there's a way to start re-submit the original modules for a compliance check with the new requirements. Although the new proposal (for Type Hints and static type checking) might make that necessary again in the near future.

https://sourceforge.net/p/gramps/mailman/gramps-devel/thread/46818942-927a-463d-ba3c-4d04de3aaf41%40gramps-project.org/#msg58766730

Nick-Hall

This looks good. I just made a few suggestions.

To get rid of the formatting errors, run black on the files you changed.

gramps/gui/filters/sidebar/_citationsidebarfilter.py

Nick-Hall · 2024-05-11T19:56:31Z

gramps/gui/filters/sidebar/_sidebarfilter.py

+                rule.list[2] = str(rule.list[2])
+            new_rules.append(rule)
+        the_filter.flist = new_rules
+        comment = "Created by Filter gramplet on " + time.strftime("%Y-%m-%d", time.localtime(time.time()))


This string looks like it should be translated. Should we use the current locale settings for the date format?

I've pushed a commit to fix this. I think we can leave the date in ISO format.

Nick-Hall · 2024-05-11T20:03:28Z

gramps/gui/filters/sidebar/_sidebarfilter.py

+            if isinstance(rule, gramps.gen.filters.rules.place.WithinArea):
+                if rule.list[0] is None: # No active place
+                    msg = _("You should select a place when using the 'Within' rule")
+                    self.msg_label.set_markup("<span color='red'>" + msg + "</span>")


It is better to add the css error class to the widget style context rather than hard-coding the colour "red". For example you could use:

self.msg_label.set_text(msg) self.msg_label.get_style_context().add_class("error")

This will use the error colour defined in the theme. This is useful for visually impaired users and non-European cultures.

I don't quite understand how css and themes work in Gramps. If I add the "error" class then the label still doesn't change color.

Interesting. I've pushed a commit to your branch which works for me. The errors are red, but the elapsed time is black.

I can make the css style work if I manually make changes in the file ~/.config/gtk-3.0/gtk.css. But that is probably not the correct the way to do this, is it?

Anyway, it seems that we are doing changes simultaneously. Did I mess things up?

What operating system and theme are you using? No changes to the css should be necessary.

When you run Gramps in the master branch, what colour is the version number in the status bar? That is set using the "destructive-action" class.

You can either use my changes or update the code yourself.

Nick-Hall · 2024-05-11T20:03:37Z

gramps/gui/filters/sidebar/_sidebarfilter.py

+        self.filterdb = gramps.gen.filters.CustomFilters
+        the_filter = self.get_filter()
+        if the_filter is None:
+            self.msg_label.set_markup("<span color='red'>" + _("Supply at least one value") + "</span>")


Nick-Hall · 2024-05-11T20:08:46Z

gramps/gen/filters/rules/event/__init__.py

@@ -71,4 +72,5 @@
    HasTag,
    MatchesPlaceFilter,
    HasDayOfWeek,
+    HasEventBase,


How is HasEventBase and HasSourceBase related to this PR? I can't see them referenced anywhere.

The 'Define filter' feature does not work without it.

Event filtering uses the rule class HasEventBase and 'Define filter' will store that class name in custom_filters.xml. For example:

<object type="Event"> <filter name="test" function="and" comment="Created by Filter gramplet on 2024-05-12"> <rule class="HasEventBase" use_regex="False" use_case="False"> ... </rule> </object>

However, when custom_filters.xml is read and interpreted then the corresponding rule is silently ignored because the class is not imported in gramps.gen.filters.rules.event.__init__.py. There is only this message on the console:

ERROR: Filter rule 'HasEventBase' in filter 'test' not found!

My solution is to import the class in the __init__.py file. At first i thought it also needed to be added in 'editor_rule_list' but that does not seem to be necessary.

I don't see any warnings or errors if I comment these lines out.

The HasEventBase is used as the "Events with parameters" rule. It works if I click the 'Define filter' button in the sidebar, or if I invoke the filter editor from the menu.

emyoulation · 2024-05-12T16:42:15Z

Kari,

Could you add your 2024 copyright to the list of contributors for these 5 files? It isn't just about taking credit. It also agrees to the terms of the licensing for your contribution. And it also builds confidence by showing that the modules are continuing to evolve. Most of these show 2010 as the last touch.

Cross-link to Discourse forum discussion:
https://gramps.discourse.group/t/can-add-on-gramplets-use-the-lib-plugin-type-to-supersede-built-ins/5422/10

MantisBT request 0013289: Roll Filter+ gramplet plugins into the core (built-in) plugins for 5.3

kkujansuu · 2024-05-12T18:02:56Z

I am using Linux Mint and I don't know what theme I am using! How to check that? su 12. toukok. 2024 klo 20.56 Nick Hall ***@***.***) kirjoitti:

…

***@***.**** commented on this pull request. ------------------------------ In gramps/gui/filters/sidebar/_sidebarfilter.py <#1721 (comment)> : > @@ -316,3 +363,47 @@ def set_filters_to_name(self, filter_name): self.generic.set_active_iter(iter) break iter = liststore.iter_next(iter) + + def filter_is_ok(self): + the_filter = self.get_filter() + if the_filter is None: + return True + for rule in the_filter.get_rules(): + if isinstance(rule, gramps.gen.filters.rules.place.WithinArea): + if rule.list[0] is None: # No active place + msg = _("You should select a place when using the 'Within' rule") + self.msg_label.set_markup("" + msg + "") What operating system and theme are you using? No changes to the css should be necessary. When you run Gramps in the *master* branch, what colour is the version number in the status bar? That is set using the "destructive-action" class. You can either use my changes or update the code yourself. — Reply to this email directly, view it on GitHub <#1721 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AEIYOXQTJLMKW5ZYEC4IMXTZB6UM7AVCNFSM6AAAAABHSDFO3WVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDANJRGQZDIMBSHE> . You are receiving this because you authored the thread.Message ID: ***@***.***>

kkujansuu · 2024-05-12T18:17:07Z

Yes, it is sufficient to have HasEventBase imported in the module. I noticed that also. su 12. toukok. 2024 klo 20.59 Nick Hall ***@***.***) kirjoitti:

…

***@***.**** commented on this pull request. ------------------------------ In gramps/gen/filters/rules/event/__init__.py <#1721 (comment)> : > @@ -71,4 +72,5 @@ HasTag, MatchesPlaceFilter, HasDayOfWeek, + HasEventBase, I don't see any warnings or errors if I comment these lines out. The HasEventBase is used as the "Events with parameters" rule. It works if I click the 'Define filter' button in the sidebar, or if I invoke the filter editor from the menu. — Reply to this email directly, view it on GitHub <#1721 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AEIYOXSQT6XQMQEHN6URQ7TZB6UZTAVCNFSM6AAAAABHSDFO3WVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDANJRGQZDIMZWHA> . You are receiving this because you authored the thread.Message ID: ***@***.***>

Nick-Hall · 2024-05-12T20:14:22Z

I am using Linux Mint and I don't know what theme I am using! How to check that?

From the command line, start Gramps with:

GTK_DEBUG=interactive python Gramps.py

Then, in the debugger, look in the "Visual" tab.

Note: The Adwaita error colour is (204, 0, 0). The colour "red" is (255, 0, 0).

kkujansuu · 2024-05-13T06:03:48Z

GTK Theme = Mint-Y su 12. toukok. 2024 klo 23.14 Nick Hall ***@***.***) kirjoitti:

…

I am using Linux Mint and I don't know what theme I am using! How to check that? From the command line, start Gramps with: GTK_DEBUG=interactive python Gramps.py Then, in the debugger, look in the "Visual" tab. — Reply to this email directly, view it on GitHub <#1721 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AEIYOXS7DDKJJ35MOYQR4GLZB7ETLAVCNFSM6AAAAABHSDFO3WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMBWGM3DEOBUGY> . You are receiving this because you authored the thread.Message ID: ***@***.***>

kkujansuu · 2024-05-13T12:13:03Z

The version number is red. If I add the "destructive-action" class to the Reset button then it turns red. But using it on the label does not work. The "error" class does not work for the button either. su 12. toukok. 2024 klo 20.56 Nick Hall ***@***.***) kirjoitti:

…

***@***.**** commented on this pull request. ------------------------------ In gramps/gui/filters/sidebar/_sidebarfilter.py <#1721 (comment)> : > @@ -316,3 +363,47 @@ def set_filters_to_name(self, filter_name): self.generic.set_active_iter(iter) break iter = liststore.iter_next(iter) + + def filter_is_ok(self): + the_filter = self.get_filter() + if the_filter is None: + return True + for rule in the_filter.get_rules(): + if isinstance(rule, gramps.gen.filters.rules.place.WithinArea): + if rule.list[0] is None: # No active place + msg = _("You should select a place when using the 'Within' rule") + self.msg_label.set_markup("" + msg + "") What operating system and theme are you using? No changes to the css should be necessary. When you run Gramps in the *master* branch, what colour is the version number in the status bar? That is set using the "destructive-action" class. You can either use my changes or update the code yourself. — Reply to this email directly, view it on GitHub <#1721 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AEIYOXQTJLMKW5ZYEC4IMXTZB6UM7AVCNFSM6AAAAABHSDFO3WVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDANJRGQZDIMBSHE> . You are receiving this because you authored the thread.Message ID: ***@***.***>

Nick-Hall · 2024-05-13T17:14:53Z

GTK Theme = Mint-Y

The Mint-Y theme doesn't seem to define the "error" class for labels, although the Mint-X theme does. The "destructive-action" class is only used by buttons.

I suggest that we add the following line to our gramps.css file:

label.error {color: @error_color}

This will let us use classes as recommended. I expect that the @error_color will be used by themes anyway, so overriding it in this way shouldn't cause a problem.

emyoulation · 2024-05-13T22:04:30Z

Have you tried the GTK+ Inspector tool? It was very enlightening when exploring what CSS was applied to each element of the Gramps GUI.

For Linux only. Wish there were Windows and macOS versions.

Nick-Hall · 2024-05-13T23:31:32Z

Have you tried the GTK+ Inspector tool?

What happens if you try to start it in Windows?

emyoulation · 2024-05-14T01:00:20Z

Have you tried the GTK+ Inspector tool?
What happens if you try to start it in Windows?

I only found GTK+ Inspector in the Gnome project. Haven't found info about installation for Windows or macOS

jralls · 2024-05-14T02:59:47Z

The inspector is part of Gtk. Just start Gramps from the command line with e.g. GTK_DEBUG=interactive gramps and it pops up behind the gramps window. On Windows you probably have to do it in two lines, e.g.

set GTK_DEBUG interactive
gramps

There are also two keyboard shortcuts, <ctrl><shift>I and <ctrl><shift>D, to open it when $GTK_DEBUG isn't set. They don't work on macOS but might on Windows.

kkujansuu · 2024-05-14T05:03:30Z

I am travelling this week and can't work on this. Please proceed with the PR if you wish. Anyway, I don't know what I should do next. la 11. toukok. 2024 klo 23.11 Nick Hall ***@***.***) kirjoitti:

…

***@***.**** requested changes on this pull request. This looks good. I just made a few suggestions. To get rid of the formatting errors, run black <https://black.readthedocs.io/en/stable/index.html> on the files you changed. ------------------------------ In gramps/gui/filters/sidebar/_citationsidebarfilter.py <#1721 (comment)> : > @@ -78,7 +78,7 @@ def __init__(self, dbstate, uistate, clicked): for conf_value in sorted(conf_strings.keys()): model.append((_(conf_strings[conf_value]),)) self.filter_conf.set_model(model) - self.filter_conf.set_active(Citation.CONF_NORMAL) + self.filter_conf.set_active(Citation.CONF_VERY_LOW) This looks like a duplicate of PR #1701 <#1701>. ------------------------------ In gramps/gui/filters/sidebar/_sidebarfilter.py <#1721 (comment)> : > + if the_filter is None: + self.msg_label.set_markup("" + _("Supply at least one value") + "") + return + + if not self.filter_is_ok(): + return + # fix some rules: + new_rules = [] + for rule in the_filter.get_rules(): + # The Place rule WithinArea might have numeric values while custom_filters.xml only accepts strings. + if isinstance(rule, gramps.gen.filters.rules.place.WithinArea): + rule.list[1] = str(rule.list[1]) + rule.list[2] = str(rule.list[2]) + new_rules.append(rule) + the_filter.flist = new_rules + comment = "Created by Filter gramplet on " + time.strftime("%Y-%m-%d", time.localtime(time.time())) This string looks like it should be translated. Should we use the current locale settings for the date format? ------------------------------ In gramps/gui/filters/sidebar/_sidebarfilter.py <#1721 (comment)> : > @@ -316,3 +363,47 @@ def set_filters_to_name(self, filter_name): self.generic.set_active_iter(iter) break iter = liststore.iter_next(iter) + + def filter_is_ok(self): + the_filter = self.get_filter() + if the_filter is None: + return True + for rule in the_filter.get_rules(): + if isinstance(rule, gramps.gen.filters.rules.place.WithinArea): + if rule.list[0] is None: # No active place + msg = _("You should select a place when using the 'Within' rule") + self.msg_label.set_markup("" + msg + "") It is better to add the css error class to the widget style context rather than hard-coding the colour "red". For example you could use: self.msg_label.set_text(msg) self.msg_label.get_style_context().add_class("error") This will use the error colour defined in the theme. This is useful for visually impaired users and non-European cultures. ------------------------------ In gramps/gui/filters/sidebar/_sidebarfilter.py <#1721 (comment)> : > + the_filter = self.get_filter() + if the_filter is None: + return True + for rule in the_filter.get_rules(): + if isinstance(rule, gramps.gen.filters.rules.place.WithinArea): + if rule.list[0] is None: # No active place + msg = _("You should select a place when using the 'Within' rule") + self.msg_label.set_markup("" + msg + "") + return False + return True + + def define_filter(self, _obj): + self.filterdb = gramps.gen.filters.CustomFilters + the_filter = self.get_filter() + if the_filter is None: + self.msg_label.set_markup("" + _("Supply at least one value") + "") As above. ------------------------------ In gramps/gen/filters/rules/event/__init__.py <#1721 (comment)> : > @@ -71,4 +72,5 @@ HasTag, MatchesPlaceFilter, HasDayOfWeek, + HasEventBase, How is HasEventBase and HasSourceBase related to this PR? I can't see them referenced anywhere. — Reply to this email directly, view it on GitHub <#1721 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AEIYOXWT463UNOTQL4EI5OLZBZ3NNAVCNFSM6AAAAABHSDFO3WVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDANJRGI2TCNZQG4> . You are receiving this because you authored the thread.Message ID: ***@***.***>

Nick-Hall · 2024-05-14T14:11:44Z

@kkujansuu I'll finish this for you.

Nick-Hall added the enhancement label May 11, 2024

Nick-Hall requested changes May 11, 2024

View reviewed changes

This was referenced May 11, 2024

Change default confidence level in citation filter #1701

Closed

Update citation.py #1700

Closed

Nick-Hall force-pushed the master branch from 230e6de to a4bec00 Compare May 12, 2024 16:30

kkujansuu and others added 4 commits May 14, 2024 18:42

Enhanced version of the Filter gramplet

a473ad0

Use css error class rather then hardcoding the colour "red"

3310f75

Translate the filter comment string

7eed306

Add new HasEvent and HasSource rules

b80d971

Nick-Hall force-pushed the master branch from a4bec00 to b80d971 Compare May 14, 2024 20:46

cdhorn mentioned this pull request May 16, 2024

Some filter rule type related cleanups #1533

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhanced version of the Filter gramplet #1721

Enhanced version of the Filter gramplet #1721

kkujansuu commented May 11, 2024

emyoulation commented May 11, 2024 •

edited

kkujansuu commented May 11, 2024 via email •

edited

emyoulation commented May 11, 2024 •

edited

Nick-Hall left a comment

Nick-Hall May 11, 2024

Nick-Hall May 12, 2024

Nick-Hall May 11, 2024

kkujansuu May 12, 2024

Nick-Hall May 12, 2024

kkujansuu May 12, 2024

kkujansuu May 12, 2024

Nick-Hall May 12, 2024

Nick-Hall May 11, 2024

Nick-Hall May 11, 2024

kkujansuu May 12, 2024 •

edited by Nick-Hall

Nick-Hall May 12, 2024

emyoulation commented May 12, 2024

kkujansuu commented May 12, 2024 via email

kkujansuu commented May 12, 2024 via email

Nick-Hall commented May 12, 2024 •

edited

kkujansuu commented May 13, 2024 via email

kkujansuu commented May 13, 2024 via email

Nick-Hall commented May 13, 2024

emyoulation commented May 13, 2024

Nick-Hall commented May 13, 2024

emyoulation commented May 14, 2024

jralls commented May 14, 2024

kkujansuu commented May 14, 2024 via email

Nick-Hall commented May 14, 2024

Enhanced version of the Filter gramplet #1721

Are you sure you want to change the base?

Enhanced version of the Filter gramplet #1721

Conversation

kkujansuu commented May 11, 2024

emyoulation commented May 11, 2024 • edited

kkujansuu commented May 11, 2024 via email • edited

emyoulation commented May 11, 2024 • edited

Nick-Hall left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kkujansuu May 12, 2024 • edited by Nick-Hall

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emyoulation commented May 12, 2024

kkujansuu commented May 12, 2024 via email

kkujansuu commented May 12, 2024 via email

Nick-Hall commented May 12, 2024 • edited

kkujansuu commented May 13, 2024 via email

kkujansuu commented May 13, 2024 via email

Nick-Hall commented May 13, 2024

emyoulation commented May 13, 2024

Nick-Hall commented May 13, 2024

emyoulation commented May 14, 2024

jralls commented May 14, 2024

kkujansuu commented May 14, 2024 via email

Nick-Hall commented May 14, 2024

emyoulation commented May 11, 2024 •

edited

kkujansuu commented May 11, 2024 via email •

edited

emyoulation commented May 11, 2024 •

edited

kkujansuu May 12, 2024 •

edited by Nick-Hall

Nick-Hall commented May 12, 2024 •

edited