Render plain text README.txt monospaced #2721

jonasfranz · 2017-10-16T20:33:33Z

Will add <pre>TEXT</pre> at the readme page.

Screenshot

Signed-off-by: Jonas Franz <[email protected]>

codecov-io · 2017-10-16T20:43:29Z

Codecov Report

Merging #2721 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #2721   +/-   ##
=======================================
  Coverage   26.96%   26.96%           
=======================================
  Files          87       87           
  Lines       17297    17297           
=======================================
  Hits         4664     4664           
  Misses      11953    11953           
  Partials      680      680

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 33647aa...030ae40. Read the comment docs.

Signed-off-by: Jonas Franz <[email protected]>

Morlinest · 2017-10-16T20:51:28Z

This will break markdown rendering.

jonasfranz · 2017-10-16T20:52:49Z

@Morlinest Why? It is not used when markup is rendered. (See acaf4b3)

Morlinest · 2017-10-16T21:00:03Z

@JonasFranzDEV I was checking your first version (commit). Will look at it again.

Morlinest · 2017-10-16T21:02:54Z

routers/repo/view.go

 			ctx.Data["FileContent"] = string(markup.Render(blob.Name(), buf, path.Dir(treeLink), ctx.Repo.Repository.ComposeMetas()))
 		} else if readmeExist {
 			ctx.Data["IsRenderedHTML"] = true
+			ctx.Data["IsMarkup"] = false


You can keep only ctx.Data["IsMarkup"] = true

Morlinest · 2017-10-16T21:03:37Z

templates/repo/view_file.tmpl

@@ -38,7 +38,11 @@
 	<div class="ui attached table segment">
 		<div class="file-view {{if .IsRenderedHTML}}markdown{{else if .IsTextFile}}code-view{{end}} has-emoji">
 			{{if .IsRenderedHTML}}
-				<pre>{{if .FileContent}}{{.FileContent | Str2html}}{{end}}</pre>
+				{{if not .IsMarkup}}


Switch order, test for {{if .IsMarkup}}

Signed-off-by: Jonas Franz <[email protected]>

Morlinest · 2017-10-16T21:20:39Z

templates/repo/view_file.tmpl

+				{{if .IsMarkup}}
+					{{if .FileContent}}{{.FileContent | Str2html}}{{end}}
+				{{else}}
+					<pre>{{if .FileContent}}{{.FileContent | Str2html}}{{end}}</pre>


Is rly Str2html needed here?
<pre> should be inside {{if .FileContent}} block.

Edit: Probably it's safe to always use Str2html

Morlinest · 2017-10-16T21:24:30Z

routers/repo/view.go

@@ -195,6 +195,7 @@ func renderFile(ctx *context.Context, entry *git.TreeEntry, treeLink, rawLink st
 		ctx.Data["ReadmeExist"] = readmeExist
 		if markup.Type(blob.Name()) != "" {
 			ctx.Data["IsRenderedHTML"] = true
+			ctx.Data["IsMarkup"] = true


Please look for other places with markup.Type check. I think it (ctx.Data["IsMarkup"] = true) should be added also there (I found https://github.com/JonasFranzDEV/gitea/blob/5aca5a8a3d618630d10272445badcbea5381683f/routers/repo/view.go#L97).

lafriks · 2017-10-16T21:26:14Z

templates/repo/view_file.tmpl

@@ -38,7 +38,11 @@
 	<div class="ui attached table segment">
 		<div class="file-view {{if .IsRenderedHTML}}markdown{{else if .IsTextFile}}code-view{{end}} has-emoji">


Please add here {{.IsMarkup}}markdown{{else}}plain-text{{end}} in place of markdown and in _repository.less file after line 271 add style:

.plain-text { padding: 1em 2em 1em 2em; }

I think this way looks much better:

This looks actually like this:

Is this still okay? I've left the <pre> stuff in place.

EDIT:
It looks fine Chrome (I've used safari). Maybe it is a cache problem in Safari.

Should be fine in all, try cleaning cache (also did you do make generate-stylesheets?)

also did you do make generate-stylesheets?

Yes

Morlinest · 2017-10-16T21:35:36Z

routers/repo/view.go

@@ -195,6 +195,7 @@ func renderFile(ctx *context.Context, entry *git.TreeEntry, treeLink, rawLink st
 		ctx.Data["ReadmeExist"] = readmeExist
 		if markup.Type(blob.Name()) != "" {
 			ctx.Data["IsRenderedHTML"] = true


ctx.Data["IsRenderedHTML"] = true can be removed (check other places, same as markup.Type) if ctx.Data["IsMarkup"] = true is set, then you can remove nested if from template and use else if.

Where should I remove the if in template? (Could you provide a Line?)

https://github.com/go-gitea/gitea/pull/2721/files#diff-2ec327528ed2be8dd3c2201ef8e6a42cR40

Signed-off-by: Jonas Franz <[email protected]>

lafriks · 2017-10-16T21:57:36Z

public/less/_repository.less

@@ -270,6 +270,10 @@
 				}
 			}

+			.plain-text {
+            	padding: 1em 2em 1em 2em;


Something looks wrong with spacing

Fixed spacing at repository.less Signed-off-by: Jonas Franz <[email protected]>

Morlinest · 2017-10-16T22:16:29Z

routers/repo/view.go

@@ -95,6 +95,7 @@ func renderDirectory(ctx *context.Context, treeLink string) {
 			buf = append(buf, d...)
 			ctx.Data["IsRenderedHTML"] = true


Move this line to line 101

Signed-off-by: Jonas Franz <[email protected]>

lafriks · 2017-10-16T22:35:29Z

LGTM

Morlinest · 2017-10-16T22:49:38Z

templates/repo/view_file.tmpl

 				{{if .FileContent}}{{.FileContent | Str2html}}{{end}}
+			{{else if .IsRenderedHTML}}
+				<pre>{{if .FileContent}}{{.FileContent | Str2html}}{{end}}</pre>


Move <pre> inside {{if .FileContent}} block. Don't need to render empty tag.

@Morlinest it is better to render it because of margins as css is accounted for plain-text to have pre inside

@lafriks OK then

Morlinest · 2017-10-16T22:56:50Z

LGTM

Render plain text README.txt monospaced

32f3d8c

Signed-off-by: Jonas Franz <[email protected]>

tboerger added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Oct 16, 2017

Removing monospaced text at markup READMEs

acaf4b3

Signed-off-by: Jonas Franz <[email protected]>