8000 Fix tab indentation in code view by cipherboy · Pull Request #10671 · go-gitea/gitea · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix tab indentation in code view #10671

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
Merged
merged 2 commits into from
Mar 25, 2020
Merged

Fix tab indentation in code view #10671

merged 2 commits into from
Mar 25, 2020

Conversation

cipherboy
Copy link
Contributor

Resolves: #5596

Signed-off-by: Alexander Scheel <alexander.m.scheel@gmail.com>

On my system (F31, Firefox 73), this almost works. It does move the code over by one character, without having the prefixed space affect the tab indentation. The issue is that Firefox can't seem to correctly calculate the offset of the size of 8 spaces in the selected font (Liberation Mono).

See attached screenshots:

Screenshot from 2020-03-08 11-59-36

When this font is disabled, it does work:

Screenshot from 2020-03-08 11-59-47

We can probably leave the width as-is, I just updated it to keep the same original width.

@cipherboy
Fix tab indentation in code view
eb09077 
Resolves: go-gitea#5596

Signed-off-by: Alexander Scheel <alexander.m.scheel@gmail.com>
@codecov-io
Copy link

Codecov Report

Merging #10671 into master will increase coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #10671      +/-   ##
==========================================
+ Coverage    43.6%   43.61%   +0.01%     
==========================================
  Files         588      588              
  Lines       82485    82485              
==========================================
+ Hits        35965    35974       +9     
+ Misses      42059    42048      -11     
- Partials     4461     4463       +2
Impacted Files Coverage Δ
modules/indexer/stats/db.go 50% <0%> (-9.38%) ⬇️
modules/queue/unique_queue_disk_channel.go 53.84% <0%> (-1.93%) ⬇️
modules/queue/workerpool.go 56.93% <0%> (-1.07%) ⬇️
services/pull/pull.go 35% <0%> (-0.89%) ⬇️
modules/repofiles/update.go 38.69% <0%> (-0.39%) ⬇️
modules/setting/git.go 65.38% <0%> (ø) ⬆️
routers/private/hook.go 36.6% <0%> (ø) ⬆️
models/error.go 29.23% <0%> (+0.51%) ⬆️
models/notification.go 64.71% <0%> (+0.83%) ⬆️
models/unit.go 39.5% <0%> (+2.46%) ⬆️
... and 3 more

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 4901096...eb09077. Read the comment docs.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 8, 2020
@guillep2k guillep2k added the topic/ui Change the appearance of the Gitea UI label Mar 8, 2020
@guillep2k guillep2k added this to the 1.12.0 milestone Mar 8, 2020
@cipherboy
Copy link
Contributor Author

FWIW, I can reproduce the font rendering issue in Firefox when viewing the raw file. So, this is a Firefox/Linux/Liberation Mono bug. This PR can continue.

Screenshot from 2020-03-10 12-17-27

@zeripath
Copy link
Contributor

In which case we should remove Liberation Mono from the suggested fonts I think.

6543
6543 approved these changes Mar 11, 2020
View reviewed changes
Copy link
Member
@6543 6543 left a comment

Choose a reason for hiding this comment

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

all in all - I like the change of this pull

@GiteaBot GiteaBot 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 Mar 11, 2020
@cipherboy
Copy link
Contributor Author
cipherboy commented Mar 12, 2020 •
edited
Loading

@zeripath said:

In which case we should remove Liberation Mono from the suggested fonts I think.

I've filed a Firefox bug about it. I've tried two different versions of Liberation Mono from Fedora and they both seem to have this problem. There's also a historical Fedora bug, but that's been marked as resolved and I can't reproduce this issue in gedit or Chromium. It seems to only be a Firefox problem for some reason.

I'd prefer to leave it in the list for now. I think Liberation Mono is a fairly common font on Linux systems.

@GiteaBot GiteaBot 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 Mar 25, 2020
@zeripath zeripath closed this Mar 25, 2020
@zeripath zeripath reopened this Mar 25, 2020
@zeripath
Copy link
Contributor

make lg-tm work

@zeripath zeripath merged commit 61830ac into go-gitea:master Mar 25, 2020
@cipherboy
Copy link
Contributor Author

Thanks for the merge!

@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@mrsdizzie mrsdizzie mrsdizzie approved these changes

@6543 6543 6543 approved these changes

Assignees
No one assigned
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. topic/ui Change the appearance of the Gitea UI
Projects
None yet
Milestone
1.12.0
Development

Successfully merging this pull request may close these issues.

First indentation is shorter by one character
8 participants
@cipherboy @codecov-io @zeripath @mrsdizzie @6543 @guillep2k @GiteaBot @lunny
0