Filter Strings for Deck Editor search #3582

basicer · 2019-02-18T07:55:29Z

Related Issues

Fixes New Feature: Sort and filter by Format #215

Short roundup of the initial problem

Searching cards in the deck editor is hard.

What will change with this Pull Request?

Entering a string in the search box with [:=<>] in it will try to parse the query in the MagicCards.info syntax.

Screenshots

tooomm · 2019-02-18T12:33:45Z

MagicCards.info syntax

You mean scryfall syntax since they took over mci? https://scryfall.com/docs/syntax

If it's the old mci syntax and that's any different, I wouldn't use that since the site no longer exists and people can't check the syntax or new users are not used to it at all.

In general a very nice new feature though! 👍

ZeldaZach · 2019-02-18T21:49:16Z

My question is: Why doesn't this auto-create the filters in the filter section once they're typed up? Would make a bit more sense as why would we have two types of filtering operations?

basicer · 2019-02-18T21:56:09Z

I tried generating the filter tree in v0, but it turned out the filter tree wasn't expressive enough to capture the kinds of queries you can write.

basicer · 2019-02-18T23:05:46Z

MagicCards.info syntax

You mean scryfall syntax since they took over mci? https://scryfall.com/docs/syntax

If it's the old mci syntax and that's any different, I wouldn't use that since the site no longer exists and people can't check the syntax or new users are not used to it at all.

In general a very nice new feature though! 👍

What I really mean is http://magidex.com syntax : )

ebbit1q · 2019-02-19T00:27:06Z

Scryfall syntax is almost exactly the same: https://scryfall.com/docs/syntax
Exception is the + and operation on sets (+ is or on Scryfall instead)
Also Scryfall is more expansive, with more details going as far as what character is represented on the art of the card.

basicer · 2019-02-19T00:49:01Z

The joke is I made magidex.com before scryfall ;)

cockatrice/src/filter_string.cpp

ZeldaZach · 2019-02-20T08:43:42Z

cockatrice/src/filter_string.cpp

+            auto op = sv[0].get<QString>();
+            return [=](int s) {
+                if (op == ">")
+                    return s > arg;


I remember in the past for a school problem I was able to convert the op string to a real op without switches... maybe I'm mistaken?

cockatrice/src/filter_string.cpp

ZeldaZach

Seems fine to me. More of an Easter egg than anything, but has no detriment to the program so I'm fine having it.

ZeldaZach · 2019-02-21T18:46:57Z

cockatrice/src/filter_string.cpp

+
+std::once_flag init;
+
+FilterString::FilterString(const QString &expr)


Would really like some comments in here

basicer · 2019-02-21T19:04:17Z

Seems fine to me. More of an Easter egg than anything, but has no detriment to the program so I'm fine having it.

Should I add a tip of the day?

ZeldaZach · 2019-02-21T19:04:28Z

Good idea, forgot about that 😅

ebbit1q · 2019-02-21T20:56:42Z

hold on, do we now have two separate filter methods? we already have a entire separate filter widget and all.

ctrlaltca · 2019-02-21T21:08:56Z

I agree, having two different ways to do the same thing (filtering cards in the deck editor), both of them not exactly clear to use and not well documented is not really good from an user perspective.
To be honest, this implementation looks far better than the existing one; it would be nice to merge them into a single one or even drop the older, as long as we implement a visual way to discover the possible syntax that can be user in the new filter.

basicer · 2019-02-21T21:21:24Z

Could add something like this to the search text widget that gives syntax help.

ebbit1q · 2019-02-21T21:23:28Z

I don't like the old filter system either. this would solve multiple issues with it. Documenting it well is a must.

ctrlaltca · 2019-02-21T21:24:26Z

That would be great. Also, a history of the last used search queries, so that a recent search pattern can be reused easily. That would totally surpass the old filters capabilities and make it obsolete.

tests/carddatabase/filter_string_test.cpp

ebbit1q

The parser isn't activated if a search only contains quotes"".
This means the search for "Birds of Paradise" has no results, but the search "Birds of Paradise" t:bird does what you'd expect.

This search overrules the filter widget, so setting a filter there for something like cmc = 5 and searching t:angel will still give angels of all mana costs.
If the old system is still there it should at least be told somewhere that they don't combine.

The clickable link for c:(l,c) in the syntax help has an added brace to the right, clicking it results in c:(l%2Cc being inserted in the search bar with no results. Other links with ,)+ in them don't work either.
Inserting c:(l,c) doesn't actually work either. Other searches with (),+ in them don't work either.

The format filters f: and banned: don't actually work, unsurprisingly as this data isn't stored in xmls afaik. They should be removed from the syntax help.

The filter for rarity r: doesn't work even though mentioned in the syntax help.

Searching for o:{T} doesn't work, o:"{T}" does work, this should be changed in the syntax help.

Searching for c:wu doesn't work, nor does any combined letter combination, single letters as c:w work fine.

Searching for c!w doesn't work, nor does the ! syntax in any combination.

ebbit1q

There still are a few things in the syntax guide that don't work as expected:

while c:m does work c:mw or c:wm ignores m and filters exactly like c:w
c!uw actually only tests if a card is both
c!uwm gives no results at all
c= doesn't work at all and gets mojibaked when clicked as well, just like ! used to
, doesn't work, instead some form of or keyword might be better
e:lea+leb doesn't work and clicking it inserts e:al,be instead, same for e:lea,leb -e:lea+leb
** just replace e:lea+leb with e:lea e:leb

also:

e: doesn't work for sets with a number in them like 6ed or cn2
the keyword for toughness sorting isn't mentioned in the syntax help even though it's easily assumed to be tou
c!c doesn't work, in theory it should give the same results as c:c
closing the cockatrice main window should close the syntax help window as well

basicer · 2019-02-27T03:07:23Z

f: and banned: aren't implemented, it's best to remove them from the syntax help for now

They should be, update your card database?

ebbit1q · 2019-02-27T10:01:49Z

My bad, they work fine.

ebbit1q · 2019-02-27T13:31:50Z

cockatrice/src/tab_deck_editor.cpp

@@ -700,7 +704,11 @@ void TabDeckEditor::updateCardInfoRight(const QModelIndex &current, const QModel

 void TabDeckEditor::updateSearch(const QString &search)
 {
-    databaseDisplayModel->setCardName(search);
+    if (search.contains(QRegularExpression("[:<>=\"!]"))) {


Wouldn't it be a tad more efficient to store the regex instead? afaik qt does some optimization for them.

Agreed, having a const QRegex is better

ebbit1q · 2019-02-27T14:02:56Z

--- a/cockatrice/resources/help/search.md
+++ b/cockatrice/resources/help/search.md
@@ -23,7 +23,7 @@
 <dd>[c!w](#c!w) <small>(Cards that are only white)</small></dd>
 <dd>[c!wu](#c!wu) <small>(Cards that are only white or blue, or both)</small></dd>
 <dd>[c!wum](#c!wum) <small>(Cards that are only white and blue, and multicolored)</small></dd>
-<dd>[c=wubrg](#c%3Dwubrg) <small>(Cards that are all five colors)</small></dd>
+<dd>[c=wubrg](#c=wubrg) <small>(Cards that are all five colors)</small></dd>
 <dd>[c:m](#c:m) <small>(Any multicolored card)</small></dd>
 
 <dt><u>Pow</u>er, <u>Tou</u>ghness, <u>C</u>onverted <u>M</u>ana <u>C</u>ost:</dt>
@@ -41,7 +41,7 @@
 <dd>[e:lea](#e:lea)</dd>
 <dd>[e:lea,leb](#e:lea,leb) <small>(Cards that appear in Alpha or Beta)</small></dd>
 
-<dd>[e:lea+leb](#e:al,be) <small>(Cards that appear in Alpha and Beta)</small></dd>
-<dd>[e:lea,leb -e:lea+leb](#e:al,be -e:al,be) <small>(Cards that appear in Alpha or Beta but not in both editions)</small></dd>
+<dd>[e:lea+leb](#e:lea+leb) <small>(Cards that appear in Alpha and Beta)</small></dd>
+<dd>[e:lea,leb -e:lea+leb](#e:lea,leb -e:lea+leb) <small>(Cards that appear in Alpha or Beta but not in both editions)</small></dd>

ebbit1q

c:m now doesn't work at all anymore. c:uwm now does work though.

ebbit1q · 2019-02-28T09:50:53Z

Can you link the syntax help window so that it closes when you close the main window?

…ockatrice into 3582

ebbit1q · 2019-02-28T15:56:50Z

considering this will add legality to oracle, linking #3406

3582

All fixed!

ebbit1q

You're currently unable to chain multiple ors, that is the search for gruul or azorius or simic doesn't work. Note that this search does work if you use grouping: (gruul or azorius) or simic.

ebbit1q

certified grade A ebbit-proof

ZeldaZach

Still need to do one more pass through

Signed-off-by: Zach Halpern <ZaHalpern+github@gmail.com>

…ockatrice#3594) * I heard you like refactors so I put a refactor inside your refactor so you can refactor while you refactor * clangify * Update tab_deck_editor.h

Cleanup and refactor

ZeldaZach

Great change. Will launch a beta!

basicer force-pushed the string-filter branch 2 times, most recently from 3d3b7c2 to 8b8ced8 Compare February 18, 2019 11:50

ZeldaZach reviewed Feb 20, 2019

View reviewed changes

cockatrice/src/filter_string.cpp Show resolved Hide resolved

ZeldaZach reviewed Feb 20, 2019

View reviewed changes

cockatrice/src/filter_string.cpp Outdated Show resolved Hide resolved

basicer force-pushed the string-filter branch 3 times, most recently from b81f9d3 to 46154f3 Compare February 20, 2019 20:42

ZeldaZach reviewed Feb 21, 2019

View reviewed changes

cockatrice/src/filter_string.cpp Outdated Show resolved Hide resolved

ZeldaZach approved these changes Feb 21, 2019

View reviewed changes

ZeldaZach reviewed Feb 21, 2019

View reviewed changes

ebbit1q reviewed Feb 21, 2019

View reviewed changes

tests/carddatabase/filter_string_test.cpp Outdated Show resolved Hide resolved

basicer force-pushed the string-filter branch from 3cce20e to a4a8d67 Compare February 23, 2019 04:46

ZeldaZach added this to the Cockatrice 2.7.0 milestone Feb 23, 2019

basicer requested a review from ZeldaZach February 23, 2019 20:13

ebbit1q requested changes Feb 23, 2019

View reviewed changes

basicer force-pushed the string-filter branch from 863feb2 to ffa38c8 Compare February 25, 2019 09:05

ebbit1q requested changes Feb 27, 2019

View reviewed changes

ebbit1q reviewed Feb 27, 2019

View reviewed changes

More fixes

9d9b109

basicer force-pushed the string-filter branch from 52204bb to 9d9b109 Compare February 28, 2019 09:06

ebbit1q previously requested changes Feb 28, 2019

View reviewed changes

Fix c:m

8c0852c

basicer force-pushed the string-filter branch from 7498aeb to 8c0852c Compare February 28, 2019 09:50

ebbit1q added 2 commits February 28, 2019 10:52

Merge commit 'refs/pull/3582/head' of https://github.com/Cockatrice/C…

4988132

…ockatrice into 3582

set syntax help parent

41103c3

basicer added 2 commits February 28, 2019 10:45

Merge pull request #1 from ebbit1q/3582

0ec8095

3582

Fix warning

3d03164

basicer requested a review from ebbit1q February 28, 2019 21:20

ebbit1q requested changes Feb 28, 2019

View reviewed changes

ebbit1q and others added 2 commits March 1, 2019 01:55

add additional explanations to syntax help

7988736

Allow multiple ands/ors to be chained

552a873

ebbit1q approved these changes Mar 1, 2019

View reviewed changes

ZeldaZach requested changes Mar 1, 2019

View reviewed changes

ZeldaZach and others added 4 commits March 1, 2019 05:56

Cleanup and refactor

c009f77

Signed-off-by: Zach Halpern <ZaHalpern+github@gmail.com>

Move utility into guards

6cc9f66

Signed-off-by: Zach Halpern <ZaHalpern+github@gmail.com>

I heard you like refactors so I put a refactor inside your refactor (C…

678e169

…ockatrice#3594) * I heard you like refactors so I put a refactor inside your refactor so you can refactor while you refactor * clangify * Update tab_deck_editor.h

Merge pull request #2 from Cockatrice/zach_cleanup_onto_rob

1e7b6c2

Cleanup and refactor

ZeldaZach approved these changes Mar 1, 2019

View reviewed changes

ZeldaZach merged commit eb60fec into Cockatrice:master Mar 1, 2019

tooomm mentioned this pull request Mar 6, 2019

filter syntax window not translatable #3625

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Filter Strings for Deck Editor search #3582

Filter Strings for Deck Editor search #3582

basicer commented Feb 18, 2019 •

edited

tooomm commented Feb 18, 2019

ZeldaZach commented Feb 18, 2019

basicer commented Feb 18, 2019

basicer commented Feb 18, 2019

ebbit1q commented Feb 19, 2019 •

edited

basicer commented Feb 19, 2019

ZeldaZach Feb 20, 2019

ZeldaZach left a comment

ZeldaZach Feb 21, 2019

basicer commented Feb 21, 2019

ZeldaZach commented Feb 21, 2019

ebbit1q commented Feb 21, 2019

ctrlaltca commented Feb 21, 2019

basicer commented Feb 21, 2019

ebbit1q commented Feb 21, 2019

ctrlaltca commented Feb 21, 2019

ebbit1q left a comment •

edited

ebbit1q left a comment •

edited

basicer commented Feb 27, 2019

ebbit1q commented Feb 27, 2019

ebbit1q Feb 27, 2019

ZeldaZach Feb 28, 2019

ebbit1q commented Feb 27, 2019

ebbit1q left a comment •

edited

ebbit1q commented Feb 28, 2019

ebbit1q commented Feb 28, 2019

ebbit1q left a comment •

edited

ebbit1q left a comment

ZeldaZach left a comment

ZeldaZach left a comment


		std::once_flag init;

		FilterString::FilterString(const QString &expr)

Filter Strings for Deck Editor search #3582

Filter Strings for Deck Editor search #3582

Conversation

basicer commented Feb 18, 2019 • edited

Related Issues

Short roundup of the initial problem

What will change with this Pull Request?

Screenshots

tooomm commented Feb 18, 2019

ZeldaZach commented Feb 18, 2019

basicer commented Feb 18, 2019

basicer commented Feb 18, 2019

ebbit1q commented Feb 19, 2019 • edited

basicer commented Feb 19, 2019

ZeldaZach Feb 20, 2019

Choose a reason for hiding this comment

ZeldaZach left a comment

Choose a reason for hiding this comment

ZeldaZach Feb 21, 2019

Choose a reason for hiding this comment

basicer commented Feb 21, 2019

ZeldaZach commented Feb 21, 2019

ebbit1q commented Feb 21, 2019

ctrlaltca commented Feb 21, 2019

basicer commented Feb 21, 2019

ebbit1q commented Feb 21, 2019

ctrlaltca commented Feb 21, 2019

ebbit1q left a comment • edited

Choose a reason for hiding this comment

ebbit1q left a comment • edited

Choose a reason for hiding this comment

basicer commented Feb 27, 2019

ebbit1q commented Feb 27, 2019

ebbit1q Feb 27, 2019

Choose a reason for hiding this comment

ZeldaZach Feb 28, 2019

Choose a reason for hiding this comment

ebbit1q commented Feb 27, 2019

ebbit1q left a comment • edited

Choose a reason for hiding this comment

ebbit1q commented Feb 28, 2019

ebbit1q commented Feb 28, 2019

ebbit1q left a comment • edited

Choose a reason for hiding this comment

ebbit1q left a comment

Choose a reason for hiding this comment

ZeldaZach left a comment

Choose a reason for hiding this comment

ZeldaZach left a comment

Choose a reason for hiding this comment

basicer commented Feb 18, 2019 •

edited

ebbit1q commented Feb 19, 2019 •

edited

ebbit1q left a comment •

edited

ebbit1q left a comment •

edited

ebbit1q left a comment •

edited

ebbit1q left a comment •

edited