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.

Sign up for GitHub

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

Jump to bottom

Update documentation #1923

Merged
merged 2 commits into from
May 12, 2018
Merged

Update documentation #1923

merged 2 commits into from
May 12, 2018

Conversation

kblohm
Copy link
Contributor

@kblohm kblohm commented May 11, 2018

Hi,

  • i updated FSFormatting and added warnings in all the templates to show obsolete modules, types etc.
  • i shortened the obsolete message so that it fits on one line (looks better)
  • i changed all of the blockquotes to proper warnings/info blocks
  • i added prism.js to format non fsharp code (mostly bash, but also some json and xml). I did not manage to use it to format the console output for example here. But i am pretty sure that is due to a bug in FSFormatting. I am not able to get a <pre> without the markdown parser converting some of the elements. As far as i know markdown should not touch stuff in html-blocks so i guess that is a bug.

matthid
matthid approved these changes May 11, 2018
View reviewed changes
Copy link
Member

@matthid matthid left a comment

Choose a reason for hiding this comment

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

Thanks a lot! Overall I think we can do this just as you suggest. Just some small questions on the change to in-line html.

Also, can you please attach a screenshot on how it will look after this change (I guess you already have it running locally)?

help/markdown/contributing.md Outdated
@@ -31,15 +31,23 @@ It turns `*.md` (Markdown with embedded code snippets) and `*.fsx` files (F# scr

* Add a new git remote in order to retrieve upstream changes.

git remote add upstream https://github.com/fsharp/FAKE.git
<pre>
<code class="lang-bash">

This comment was marked as resolved.

Sign in to view

This comment was marked as resolved.

Sign in to view

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. I have to add those by hand, otherwise i get somethink like <pre class="fsnip"><code lang="bash"> and prism.js does not like those.
The ones for the commandline output are even worse. I did not manage to get them working at all so i had to leave them as is.

help/markdown/core-process.md
@@ -1,6 +1,9 @@
# Starting processes in "FAKE - F# Make"

**Note: This documentation is for FAKE 5! **
<div class="alert alert-info">
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we update the css for **? (Sorry I don't have any idea what's the best way)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I use both alert info and alert warning. Restyling bold everywhere is probably not a great idea. DocFx markdown has an extension to use notes and warnings. I could add that to FSFormatting in theory i guess.

help/templates/template.cshtml
@@ -97,7 +100,7 @@
<a href="/core.html">Core</a>
<ul>
<li><a href="/core-targets.html">Targets</a></li>
<li><a href="/apidocs/v5/fake-core-environment.html.html">Environment</a></li>
<li><a href="/apidocs/v5/fake-core-environment.html">Environment</a></li>
Copy link
Member

Choose a reason for hiding this comment

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

good eyes :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had to click on every link to see if i missed something ;)

@kblohm
Copy link
Contributor Author

kblohm commented May 11, 2018

Contributing Page:
Contributing Page

API-Docs:
API-Docs

API-Docs2:
APID-Docs2

@kblohm
Copy link
Contributor Author

kblohm commented May 11, 2018

One think that is also kinda nice with prism.js: i could just add one more javascript file and get a copy-code button.

@matthid
Copy link
Member

matthid commented May 11, 2018

Looks very nice! Good job.
One thing I noticed in the screenshots (Contribution Page) Is that the nesting of the code-blocks is no longer correct. AFAIK they should be part of the bullet points and are no longer. I'm not sure if we can fix that with the markdown/html combination. It's not a show-stopper.

@kblohm
Copy link
Contributor Author

kblohm commented May 11, 2018 •
edited
Loading

They are inside the <li>. They were not before (atleast one of the two) :p. The whole block just looks bigger because of the header. I can remove the header and place somethink on the left.
EDIT: ok i cant read.. wait i have a look

@kblohm
Copy link
Contributor Author

kblohm commented May 11, 2018

Ok i fixed the ones i could find. I will add the clipboard stuff tomorrow.
Have a good night :)

@kblohm kblohm force-pushed the updateDocsTemplate branch from 8cfb53c to e137888 Compare May 12, 2018 14:19
@kblohm
Copy link
Contributor Author

kblohm commented May 12, 2018

I fixed the wrong formatted code blocks and updated some CSS (which hates me).
I also added copy buttons to all the code snippets.
Should be good to go now.

@matthid matthid changed the base branch from master to release/rc_12 May 12, 2018 17:39
@matthid
Copy link
Member

matthid commented May 12, 2018

Ok thanks a lot! I'm trying to get this out...

@matthid matthid merged commit 73f6f5e into fsprojects:release/rc_12 May 12, 2018
@kblohm kblohm deleted the updateDocsTemplate branch May 12, 2018 20:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matthid matthid matthid approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@kblohm @matthid