kantan.csv icon indicating copy to clipboard operation
kantan.csv copied to clipboard

Add HeaderDecoder with custom header name matching

Open paulpdaniels opened this issue 3 years ago • 7 comments

Closes #315

Adds a new set of decoderWith operators to the HeaderDecoder and HeaderCodec companion objects. The old method decoder method now delegates to decoderWith but uses the default equality check for strings.

This allows users to override how the strings are compared when trying to determine the order of the headers in the mapping, e.g. when comparing the strings in a case insensitive manner.

paulpdaniels avatar Feb 09 '22 15:02 paulpdaniels

Right, this is really something I should put in the contribution guidelines. There's no issue that I can see with your PR, aside from a purely cosmetic one: the code is not formatted according to the project's guidelines.

You just need to run the scalafmtAll SBT task and it'll fix the problem(s).

nrinaudo avatar Feb 10 '22 14:02 nrinaudo

Also: given that your use case is likely quite a common one, I wouldn't be against pre-defined helpers. We have one for case-sensitive headers, why not:

  [#
  def caseInsensitiveDecoder[[#A1: CellDecoder#], R]([#f1: String#])(f: ([#A1#]) => R): HeaderDecoder[R] =
    decoderWith([#f1#])(_.equalsIgnoreCase(_))(f)#
  ]

nrinaudo avatar Feb 10 '22 15:02 nrinaudo

@nrinaudo comments addressed

paulpdaniels avatar Feb 14 '22 15:02 paulpdaniels

I, err... were they? I'm not as used to the Github UI as I should be, but checking out the latest version of the branch locally still has }} rather than }}} in GeneratedHeaderDecoders, which triggers compile time warnings (and probably disable some doctests, although I haven't checked).

nrinaudo avatar Feb 14 '22 16:02 nrinaudo

Ah ok, I switched computers because scalafmt on my windows was going crazy, so I must have lost that change. Will fix

paulpdaniels avatar Feb 15 '22 17:02 paulpdaniels

Ok I think they should be fixed now 🤞

paulpdaniels avatar Feb 20 '22 07:02 paulpdaniels

Apologies I lost track of this and #319 until I got the notification this morning. Anyway I think I addressed your comments @nrinaudo let me know if there is anything else you want on this.

paulpdaniels avatar Sep 07 '22 04:09 paulpdaniels