search

Xeverous / filter_spirit

Advanced item filter generator for Path of Exile that uses it's own DSL and online item price APIs
GNU General Public License v3.0
36 stars 7 forks source link

Rarity field name is case-sensitive #24

Closed sakurah-dev closed 4 years ago

sakurah-dev commented 4 years ago

Description Trying to compile a filter that uses a Rarity Normal will fail since it no such name exists for Normal, but using Rarity normal will work as expected

Steps to reproduce Class "Body Armour" { Rarity Normal { Show } }

Expected result Filter compiles correctly using Rarity Normal

Actual result

line 27: error: no such name exists
  Rarity Normal {
         ~~~~~~
INFO: filter generation failed

Additional context I'm guessing it would be nice to match what the actual item filter uses (Normal instead of requiring normal) or making it handle either options?

Xeverous commented 4 years ago

This is not a bug by itself, but I agree that having some filter keywords spelled differently is misleading. The original idea was to have things more closer to actual programming languages (where almost every language has lowecase keywords and literals) but after some thinking and considering not everyone is a programmer I would vote for keeping FS closer to actual filter syntax.

Making them case insensitive - that's some more work and even more if we consider unicode and all of it's corner cases.

So I will probably just edit all of these to match actual filters:

https://github.com/Xeverous/filter_spirit/blob/ba5683eb612eccb91153d7b2f15ad2dee74a5a2b/src/lib/fs/lang/keywords.hpp#L15-L119

https://github.com/Xeverous/filter_spirit/blob/ba5683eb612eccb91153d7b2f15ad2dee74a5a2b/src/lib/fs/lang/generation.hpp#L4-L80

@Alekhoff, what do you think? This would be a first breaking change, but better to do it now than later.

Xeverous commented 4 years ago

Implemented in 8c7ed3ac53aad45e59b6adda170be3f92f7d23ac.