MPD icon indicating copy to clipboard operation
MPD copied to clipboard

Added explicitly case sensitive/insensitive filter operators.

Open geneticdrift opened this issue 1 year ago • 1 comments
trafficstars

The default case sensitivity is hard coded for each command. These operators allow to override this default case sensitivity.

geneticdrift avatar Jun 29 '24 08:06 geneticdrift

Related to #2062 and #494

geneticdrift avatar Jun 29 '24 08:06 geneticdrift

This is probably a good idea, but the code doesn't look so good.

  • tryParseStringFilter() (badly named, shouldn't start with lower case) allocates memory for no reason
  • ParseStringFilter() is being refactored AND new code is being added in one single commit; this makes understanding your code very hard because I can't see what is really being changed
  • every ParseStringFilter() call allocates memory on the stack for the std::array and initializes it for no reason; if you really wanted to refactor the function to use an array, then initalize it statically (static constexpr). But since you mixed refactoring and the new feature, I don't understand why you need this array at all
  • don't concatenate std::string; this will create many unnecessary allocations. If you really want to format exception messages, use libfmt (e.g. via the FmtRuntimeError() wrapper function)

MaxKellermann avatar Jul 05 '24 14:07 MaxKellermann

The array is still not constexpr, thus it's not guaranteed to initialize at compile time; and the function still allocates heap memory.

MaxKellermann avatar Jul 08 '24 13:07 MaxKellermann