inky-rb icon indicating copy to clipboard operation
inky-rb copied to clipboard

fix raw tag parsing to work for multi-line HTML

Open SampsonCrowley opened this issue 6 years ago • 3 comments

fixes raw tag parsing mentioned in #95

SampsonCrowley avatar Apr 30 '19 03:04 SampsonCrowley

Interesting PR.

I only looked at it quickly and I'm not the final authority on this gem, but a few things come to mind:

  1. this mixes independent changes (config vs raw tag). Ideally these would be separate PRs. Also never change version numbers in a PR, that's the job of the maintainers.

  2. For the raw tag, the issue I see is that inky-rb is meant to be the Ruby port of inky, so it should probably support raw tags iff inky does.

  3. For the config part, I believe that ignoring bad values passed (e.g. test "will not set an invalid components override") is not the right way to go. Raise a TypeError instead.

marcandre avatar Apr 30 '19 05:04 marcandre

@marcandre

  1. good point, I will split it into separate PRs

  2. this repo already supports raw tags, but the current implementation in this repo only checks a single line at a time. this is an HTML based library, that's not how users expect HTML parsing to work

  3. another good point, will fix

SampsonCrowley avatar May 03 '19 00:05 SampsonCrowley

  1. Oh, right, my bad.

marcandre avatar May 03 '19 02:05 marcandre