Skip to content

Conversation

@jugglerchris
Copy link

I was sad that aoc-cli calendar was quite plain looking, and that motivated me to implement enough CSS support in html2text to fix this.

Currently the Cargo.toml in this PR points to an html2text git branch; if you're otherwise happy with this change I will merge the html2text branch, make a release, and make another update.

@scarvalhojr
Copy link
Owner

This is cool! Thank you! Yes please, make a release and we can upgrade.

pub fn show_calendar(&self) -> AocResult<()> {
let calendar_html = self.get_calendar_html()?;
let calendar_text = from_read_with_decorator(
let calendar_text = from_read_coloured(
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this is probably just a quick hack for testing but ideally we'd want this behind an optional colour flag

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could even make coloured output as the default with a flag to revert to uncoloured.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added a --no-colour-calendar option. Happy to rename etc. to anything you prefer.

@jugglerchris
Copy link
Author

I've made a release of html2text and updated Cargo.toml to point to that.

@jugglerchris
Copy link
Author

Just at that point, the calendar started displaying with extra ||||||||||||||||||, which turned out to be some animated flame implemented with position:absolute spans with CSS animation.
I've added an arguably gross hack (after adding support for display:none to html2text) to work around it.

@jugglerchris
Copy link
Author

The workaround doesn't affect the non-coloured version - depending on how you feel about it I can apply it there too.

* Use CSS as suggested by Eric Wastl to ignore the calendar animations.
* Update to html2text 0.8.0 which supports that CSS.
@jugglerchris
Copy link
Author

FWIW I've updated this MR to the latest html2text.

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.

2 participants