search

ietf-tools / svgcheck

Check SVG against RFC schema
BSD 3-Clause "New" or "Revised" License
16 stars 13 forks source link

feat: Remove command line option --no-xinclude #32

Closed kesara closed 1 year ago

kesara commented 1 year ago

Command line option --no-xinclude has no effect since the move to xml2rfc from rfctools-common.

larseggert commented 1 year ago

This breaks @martinthomson's i-d-template, because that uses -X. Please coordinate such changes more widely? (Or alternatively, keep removed options but make them no-ops?)

larseggert commented 1 year ago

It would be good to add some test cases, one of which should be the rendering of an I-D with i-d-template.

martinthomson commented 1 year ago

It's @cabo's kramdown-rfc that uses -X, I believe.

https://github.com/cabo/kramdown-rfc/blob/d8b1aa56200b0763321ecbbbaebd28f631df41f6/lib/kramdown-rfc2629.rb#L563

cabo commented 1 year ago

keep removed options but make them no-ops?)

This.

kesara commented 1 year ago

I have a PR reintroduce this with a warning because it's no-ops: https://github.com/ietf-tools/svgcheck/pull/35 The warning message will be suppressed for kramdow-rfc users because it already uses --quiet option.

cabo commented 1 year ago

Ping?

rjsparks commented 1 year ago

This is done and released isn't it? https://github.com/ietf-tools/svgcheck/releases/tag/v0.7.1

cabo commented 1 year ago

Well, svgcheck still fails, but this seems to be because it can't read from stdin.

cabo commented 1 year ago

kramdown-rfc 1.6.24 now working around the stdin issue.

cabo commented 1 year ago

Apologies for thinking we hadn't fixed this yet -- the new regression was reported to me on the same github issue the X was on. I still think that tools like this shouldn't change their CLI willy-nilly, neither losing an option (and then erroring out on it), nor losing the ability to read stdin. But there is no need for action on the latter now (from a kramdown-rfc perspective), which is why I won't raise an issue.

kesara commented 1 year ago

Sorry, It was an unintentional mistake to remove STDIN functionality. I have created #36 to restore that. Since kramdown-rfc has moved on I think it's a lesser priority issue now.