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

Markup agnostic extra files #1200

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

skalee
Copy link
Contributor

@skalee skalee commented Oct 13, 2018

Description

Rewrite how comments are handled in textual files (extra objects). Prefer markup-agnosticism over sticking to HTML comments. Do not remove comment tags, as they are bound to specific markup, and hidden from user's eye anyway. As an effect, more types of comments are now supported, including ones from AsciiDoc and Textile. Fixes #1192.

This is a somewhat breaking change (see).

Possible improvements

If required, I can restore previous behaviour changed in dc5788a, making this change a non-breaking one. Well, at least in terms of how it was formally specified in specs.

It's just a matter of adding following lines to the altered case construct:

when /^\s*$/
  cut_index_end = index
  break

See also

Completed Tasks

  • I have read the Contributing Guide.
  • The pull request is complete (implemented / written).
  • Git commits have been cleaned up (squash WIP / revert commits).
  • I wrote tests and ran bundle exec rake locally (if code is attached to PR).

@coveralls
Copy link

coveralls commented Oct 13, 2018

Coverage Status

Coverage increased (+0.004%) to 93.443% when pulling bcfb36c on skalee:markup-agnostic-extra-files into 89ea314 on lsegal:main.

@lsegal
Copy link
Owner

lsegal commented Nov 15, 2018

There are definitely concerns about breaking changes here. I realize that you show a non-breaking option, but I'm not 100% sure that dc5788a is really the only breaking change. It seems to me that all of these parser changes would break examples where, for example, a user was attempting to render an HTML document, i.e.:

<!--
# a comment
-->
<html>
<head>
...

This would potentially incorrectly render the comment in the document, depending on its contents.

This is quite an edge case, I realize, but it should be clear that this is technically a breaking change.

I actually think that the "2 newlines" option is actually the least breaking, since newlines at the top of the file are undefined behavior. Not sure if that specific heuristic is useful to your use case though.

Rewrite how comments are handled in textual files (extra objects).
Prefer markup-agnosticism over sticking to HTML comments.  Do not remove
comment tags, as they are bound to specific markup, and hidden from
user's eye anyway.

Also, see discussion in:
https://groups.google.com/forum/#!topic/yardoc/Y700TGceoII
Do not interpret attributes section of an extra file if it starts with
two blank lines (previously: one blank line).  This is a breaking
change, however not very drastical.  Seems to be a good compromise
between simplicity and ease of use.
@skalee
Copy link
Contributor Author

skalee commented Dec 12, 2019

Rebased for maintenance. Nothing has changed in commit contents.

@lsegal lsegal force-pushed the master branch 2 times, most recently from 44c07a8 to 112a890 Compare January 7, 2020 04:04
@lsegal lsegal closed this Jun 20, 2020
@lsegal lsegal reopened this Jun 20, 2020
@lsegal lsegal changed the base branch from master to main June 20, 2020 18:12
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.

Yard's meta data make AsciiDoc render ugly when used outside Yard
3 participants