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 <info@jonasfranz.software>

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 <info@jonasfranz.software>

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

 <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 <info@jonasfranz.software>

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

 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

 {{end}}
 </h4>
 <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

 readmeExist := markup.IsReadmeFile(blob.Name())
 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 <info@jonasfranz.software>

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

public/less/_repository.less

 }

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


Something looks wrong with spacing

Fixed spacing at repository.less Signed-off-by: Jonas Franz <info@jonasfranz.software>

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

routers/repo/view.go

 if isTextFile {
 d, _ := ioutil.ReadAll(dataRc)
 buf = append(buf, d...)
 ctx.Data["IsRenderedHTML"] = true


Move this line to line 101

Signed-off-by: Jonas Franz <info@jonasfranz.software>

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

LGTM

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

templates/repo/view_file.tmpl

+{{if .IsMarkup}}
 {{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 <info@jonasfranz.software>

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 <info@jonasfranz.software>

Morlinest suggested changes Oct 16, 2017

View reviewed changes

Changing order of checking .IsMarkup

5aca5a8

Signed-off-by: Jonas Franz <info@jonasfranz.software>

Morlinest suggested changes Oct 16, 2017

View reviewed changes

lafriks requested changes Oct 16, 2017

View reviewed changes

lafriks added the type/enhancement An improvement of existing functionality label Oct 16, 2017

lafriks added this to the 1.3.0 milestone Oct 16, 2017

Morlinest suggested changes Oct 16, 2017

View reviewed changes

Adding custom plain-text style

d5c9771

Signed-off-by: Jonas Franz <info@jonasfranz.software>

lafriks reviewed Oct 16, 2017

View reviewed changes

public/less/_repository.less Outdated

}

.plain-text {

padding: 1em 2em 1em 2em;

Copy link

Member

lafriks Oct 16, 2017

Choose a reason for hiding this comment

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

Something looks wrong with spacing

jonasfranz added 2 commits October 17, 2017 00:00

Optimized README renderer

5bb8520

Fixed spacing at repository.less Signed-off-by: Jonas Franz <info@jonasfranz.software>

Fix spacing

5532db8

Morlinest suggested changes Oct 16, 2017

View reviewed changes

Moving IsRenderedHTML to another position

fe00f80

Signed-off-by: Jonas Franz <info@jonasfranz.software>

lafriks approved these changes Oct 16, 2017

View reviewed changes

tboerger added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Oct 16, 2017

Morlinest suggested changes Oct 16, 2017

View reviewed changes

Morlinest approved these changes Oct 16, 2017

View reviewed changes

Merge branch 'master' into monospaced-readme-txt

030ae40

tboerger added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Oct 16, 2017

lafriks merged commit f4190f8 into go-gitea:master Oct 16, 2017

jonasfranz deleted the monospaced-readme-txt branch October 17, 2017 06:59

go-gitea locked and limited conversation to collaborators Nov 23, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Render plain text README.txt monospaced #2721

Render plain text README.txt monospaced #2721

Uh oh!

jonasfranz commented Oct 16, 2017

codecov-io commented Oct 16, 2017 •

edited

Loading

Morlinest commented Oct 16, 2017

jonasfranz commented Oct 16, 2017

Morlinest commented Oct 16, 2017

Morlinest Oct 16, 2017

Morlinest Oct 16, 2017

Morlinest Oct 16, 2017 •

edited

Loading

Morlinest Oct 16, 2017

jonasfranz Oct 16, 2017

lafriks Oct 16, 2017

jonasfranz Oct 16, 2017 •

edited

Loading

lafriks Oct 16, 2017 •

edited

Loading

jonasfranz Oct 16, 2017

Morlinest Oct 16, 2017

jonasfranz Oct 16, 2017

Morlinest Oct 16, 2017

lafriks Oct 16, 2017

Morlinest Oct 16, 2017

lafriks commented Oct 16, 2017

Morlinest Oct 16, 2017

lafriks Oct 16, 2017

Morlinest Oct 16, 2017

Morlinest commented Oct 16, 2017

Labels

5 participants

Uh oh!

Render plain text README.txt monospaced #2721

Render plain text README.txt monospaced #2721

Uh oh!

Conversation

jonasfranz commented Oct 16, 2017

Screenshot

codecov-io commented Oct 16, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Morlinest commented Oct 16, 2017

jonasfranz commented Oct 16, 2017

Morlinest commented Oct 16, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Morlinest Oct 16, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonasfranz Oct 16, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

lafriks Oct 16, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lafriks commented Oct 16, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Morlinest commented Oct 16, 2017

Labels

5 participants

codecov-io commented Oct 16, 2017 •

edited

Loading

Morlinest Oct 16, 2017 •

edited

Loading

jonasfranz Oct 16, 2017 •

edited

Loading

lafriks Oct 16, 2017 •

edited

Loading