CSS #57
CSS #57
Conversation
brettz9
force-pushed
the
css
branch
3 times, most recently
from
6d343de
to
de6fbee
Compare
|
Hoping you might still take a look at this (and more importantly, #65 ) |
|
Sorry I've left this sit so long. I suppose there's three things here:
a pull request that only addresses point 1 could probably be merged quickly Point 2, I'm less certain about for the reason mentioned here: #53 (comment). I'm not sure if we accidentally get effectively infinite nesting in some cases with or without the Point 3, the html element change here seems awkward. The use of the anchor tag has semantic problems. might impact validation and debugging tools, and might cause practical problems in some other less mainstream user agents. not sure. If the bug this is intended to fix exists on master, I think it would be better to create an issue for it, and try to address it in an independent pull request. |
brettz9
force-pushed
the
css
branch
4 times, most recently
from
7263a4b
to
65ff3ea
Compare
|
For no. 1, I've submitted #70 . If you could also look to merge what should be pretty straightforward, #67 (fixes more vulnerabilities), #68 (coverage) and #69 (linting), I think it may be easier for me to add tests for no. 2 (and/or to review/reconcile with #55). For no. 3, sure, I can add as a separate PR. But I really think it is an issue that there is no other way to workaround due to the problem with |
|
I should note that the CSS here now incorporates your changes from #55 as well as some additional. Also, FYI, in a "combined" branch (which allowed me to more easily test against nyc without adding it to this PR), I've confirmed we've gotten to 100% test coverage. However, I haven't yet split no. 2 and 3 in your list, but again, I really think there is no way to get the overriding behavior. Maybe it should be behind a config? |
|
Ok, I've updated this PR to remove the |
Also uses `const` in README and fixes typo in sp. (of "usage") in CLI help
…k>` (`<blink>` assumes a CSS keyframes at-rule that cannot be defined inline, e.g.,: `@keyframes blink {50% {opacity: 0;}}`)
- Fix: Switch from `<span>` to `<a>` (without `href`); allows `text-decoration` to be overridden (without changing to a format-breaking `inline-block`); (div set to inline also does not work as `text-decoration` is a special case); the newly renamed tests, "disables underline" and "rendering un-italic code appropriately", might not work as you wish, as they include an unnecessary disabling of underlining/italic tag, though perhaps if someone added that ANSI code, they actually wanted it (e.g., if assembling ansi together)
- Enhancement: Add rapid blink as distinct from slow blink
…ne (default) or bold-off - Enhancement: Add alternativeFonts option - Enhancement: Add fainter (2), Primary(default) font (10), 11-19 (alternative fonts), double-underline or bold-off (21), Blink off (25), Reveal (28), Not crossed out (29), Not overlined (55)
This PR offers a fix for #51 (option to display spaces). It also adds on PR #55.
I also switched to using<a>instead of<span>.The reason is that span will not allow overridingtext-decorationstyles (I had some underlining which wasn't getting disabled for this reason). (The same problem occurs in ansi-to-html's current state.)I would have thought divs would work, but when using<div style="display: inline;">, though the Inspector (in Firefox or Chrome) shows the nested values as applying (e.g., atext-decoration: none), it still shows the content with an underline. This issue appears specific totext-decoration; the following can offer some more detail: https://stackoverflow.com/questions/1823341/how-do-i-get-this-css-text-decoration-override-to-workBut things work fine in both Firefox and Chrome when I change the elements to an<a>instead (withouthref). One problem is that the browser source shows this as invalid, I guess for having both flow and phrasing content, but I still think this may be the best solution.While I may be able to go back to using spans for the colors, I'm not sure if there are any cases where resetting a color should also reset the text-decoration. And I haven't added any tests for resetting command 0 to see its effect with text-decoration.