-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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
docs: Update docsify-updated to prevent replacement #1574
Conversation
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/docsify-core/docsify-preview/HZQpUy8cb42X2S2Pne4sAZj2dLLN |
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit b0b2d48:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haha, it reminds me back to #1322. 😅
yeah, This file was previously loaded using a remote, but then rolled back... |
@@ -108,4 +108,5 @@ Current version: <span id='tip-version'>loading</span> | |||
|
|||
<script> | |||
document.getElementById('tip-version').innerText = Docsify.version | |||
document.getElementsByClassName("lang-js")[2].innerHTML = document.getElementsByClassName("lang-js")[2].innerHTML.replace(/Last modified .*'/,"Last modified {docsify-updated<span>}'</span>") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to reverse the breaking effect. Can we instead stop the breaking in the first place? It can be easier to understand that way, instead if adding "Band-Aids" that make it more difficult to understand what is happening.
Where is the break happening?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My "band aid" expression maybe sounded too harsh. It is a term my co-workers and I have used to describe quick fixes that don't solve the main issue, but add a workaround instead, making the code less ideal for the short term gain.
Do you know what is causing the issue in the first place?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because it was replaced
docsify/src/core/render/index.js
Lines 34 to 43 in 407e4d4
function formatUpdated(html, updated, fn) { | |
updated = | |
typeof fn === 'function' | |
? fn(updated) | |
: typeof fn === 'string' | |
? tinydate(fn)(new Date(updated)) | |
: updated; | |
return html.replace(/{docsify-updated}/g, updated); | |
} |
Summary
Update docsify-updated to prevent replacement
What kind of change does this PR introduce?
docs
For any code change,
Does this PR introduce a breaking change? (check one)
If yes, please describe the impact and migration path for existing applications:
Related issue, if any:
#1573
Tested in the following browsers: