Skip to content

Remove dead code for html copy starting in the middle of the line#2365

Merged
vadi2 merged 1 commit intoMudlet:developmentfrom
vadi2:remove-deadcode
Feb 23, 2019
Merged

Remove dead code for html copy starting in the middle of the line#2365
vadi2 merged 1 commit intoMudlet:developmentfrom
vadi2:remove-deadcode

Conversation

@vadi2
Copy link
Copy Markdown
Member

@vadi2 vadi2 commented Feb 22, 2019

Brief overview of PR changes/additions

Does not seem to have any effect because the variable is always set to true upon initialisation.

Motivation for adding to Mudlet

Less code!

Other info (issues closed, discussion etc)

Thanks to Coverity for reporting it.

@add-deployment-links
Copy link
Copy Markdown

add-deployment-links bot commented Feb 22, 2019

Hey there! Thanks for helping Mudlet improve. 🌟

You can directly test the changes here:

No need to install anything - just unzip and run.
Let us know if it works well, and if it doesn't, please give details.

@vadi2 vadi2 requested a review from a team as a code owner February 22, 2019 10:15
@vadi2 vadi2 requested a review from a team February 22, 2019 10:15
@vadi2 vadi2 added this to the 3.18.0 milestone Feb 22, 2019
Copy link
Copy Markdown
Member

@SlySven SlySven left a comment

Choose a reason for hiding this comment

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

🤔 yeah, that does seem to be the case.

@vadi2 vadi2 merged commit c320dc2 into Mudlet:development Feb 23, 2019
@vadi2 vadi2 deleted the remove-deadcode branch February 23, 2019 06:48
@SlySven
Copy link
Copy Markdown
Member

SlySven commented Feb 23, 2019

👣 🔫 🤦‍♂️ The purpose of the section of the code is not primarily to set the firstSpan to false but to insert a </span> if it is still true at that point (and then set it to false so is it not)... it is NOT dead code after all! What is missing is that the code introduced by #984 does NOT reset it so that the </span> IS included...

@SlySven
Copy link
Copy Markdown
Member

SlySven commented Feb 23, 2019

That is what is causing #2367 !

SlySven added a commit to SlySven/Mudlet that referenced this pull request Feb 23, 2019
A recent PR: Mudlet#2365 removed code which
was rendered ineffective by an implementation error in:
Mudlet#984 however it causes an error which
has just been raised in:
Mudlet#2367

This PR should fix the issue and it also fixes another bug that I can see
in that underlines are not being reproduced in the HTML files because of a
typo in that part of the style detail for `<span ...>` entries which used
"text-decoration: undeline" instead of: "text-decoration: underline"!

Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
SlySven added a commit that referenced this pull request Feb 24, 2019
A recent PR: #2365 removed code which
was rendered ineffective by an implementation error in:
#984 however it causes an error which
has just been raised in:
#2367

This PR should fix the issue and it also fixes another bug that I can see
in that underlines are not being reproduced in the HTML files because of a
typo in that part of the style detail for `<span ...>` entries which used
"text-decoration: undeline" instead of: "text-decoration: underline"!

Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
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