Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Jan 30, 2023

Conversation

kesara
Copy link
Member

@kesara kesara commented Jan 26, 2023

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

Command line option `--no-xinclude` has no effect since the move to
xml2rfc from rfctools-common.
@kesara kesara requested a review from rjsparks January 27, 2023 00:06
@NGPixel NGPixel changed the title feat: Remvoe command line option --no-xinclude feat: Remove command line option --no-xinclude Jan 27, 2023
@kesara kesara merged commit d3f54d9 into ietf-tools:main Jan 30, 2023
@kesara kesara deleted the feat/remove-xinclude-switch branch January 30, 2023 22:44
kesara added a commit that referenced this pull request Jan 30, 2023
Command line option `--no-xinclude` has no effect since the move to
xml2rfc from rfctools-common.
@larseggert
Copy link

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
Copy link

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
Copy link

@cabo
Copy link

cabo commented Feb 1, 2023

keep removed options but make them no-ops?)

This.

@kesara
Copy link
Member Author

kesara commented Feb 1, 2023

I have a PR reintroduce this with a warning because it's no-ops: #35
The warning message will be suppressed for kramdow-rfc users because it already uses --quiet option.

@cabo
Copy link

cabo commented Feb 22, 2023

Ping?

@rjsparks
Copy link
Member

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

@cabo
Copy link

cabo commented Feb 22, 2023

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

@cabo
Copy link

cabo commented Feb 22, 2023

kramdown-rfc 1.6.24 now working around the stdin issue.

@cabo
Copy link

cabo commented Feb 22, 2023

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
Copy link
Member Author

kesara commented Feb 22, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants