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.

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

Codemirror tabs #959

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Codemirror tabs #959

wants to merge 2 commits into from

Conversation

Mottie
Copy link
Member

@Mottie Mottie commented Jun 21, 2019

Possible fix for #543.

Test it with this gist I set up: https://gist.github.com/Mottie/d40ab58bb68cbeb2c314237189051b77/edit

2019-06-21 17_31_13-test js

@the-j0k3r the-j0k3r added the enhancement ⭐ New feature or request label Jun 22, 2019
@the-j0k3r
Copy link
Member

This works, though the dropdown settings wont work anymore. i.e this

Capture

Im not sure how helpful is to having 8 tab levels, shouldn't we reduce that to have multiples of two so 2, 4, 6, 8 No biggie just a thought.

@silverwind
Copy link
Member

2,4,8 covers 99.9% use cases.

@the-j0k3r
Copy link
Member

2,4,8 covers 99.9% use cases.

This reminds me of something I read, it said. All statistics on the web are made up on the spot =) hahaha

@the-j0k3r
Copy link
Member

the-j0k3r commented Jun 22, 2019

Should we remove/hide form-select select-sm js-code-indent-width if this is added?

Also about tab/vs spaces, can we cover that in style also, because adding this will make those GH settings not work so no point showing elements that a user is gonna fiddle with and have no effect.

IDK whats best, I dont like opinionated things as default but in this case ...

@the-j0k3r
Copy link
Member

Rebased on master to check and no dice, even the default tabs settings in GitHub are broken anyway.

@the-j0k3r
Copy link
Member

Can @StylishThemes/github please check that without GitHub Dark the tab settings while editing a file via browser, dont work.

generic

Im expecting the tab to change in existing lines this doesnt work.

With GitHub Dark the existing tab settings in usercss stylus side also dont work for this anymore either and these changes also dont work either.

@xt0rted
Copy link
Member

xt0rted commented Feb 18, 2020

@the-j0k3r those settings don't change what's already in the editor, they're for when you're writing new code. If you change to tabs that are 8 spaces wide and then add a new line to the file you should see it indented with a single tab equal to 8 spaces. That's how it works for me with and without GitHub Dark.

The tab size in the preview is also working, but there doesn't seem to be a difference between 2 and 4 for me. Switching from 4 to 8 works though.

@the-j0k3r
Copy link
Member

the-j0k3r commented Feb 18, 2020

See #543 (comment) thats how its supposed to work

The indenting for new code makes no sense without changing the indenting of existing code, thats how it used to work now with new behavior its IMO broken.

@xt0rted
Copy link
Member

xt0rted commented Feb 18, 2020

It looks like the spacing in the editor does change on existing code but only if you're using tabs. Since all our files use spaces it's not going to resize them.

@the-j0k3r
Copy link
Member

It looks like the spacing in the editor does change on existing code but only if you're using tabs. Since all our files use spaces it's not going to resize them.

Again its buggy because you have the option to select tab type. That should also matter. Its just me then =)

@the-j0k3r
Copy link
Member

the-j0k3r commented Feb 15, 2021

/* ==UserStyle==
@name           GitHub Tabs
@namespace      Tabs for GitHub
@version        1.0.0
@description    A customizable tab width style for GitHub
@version        0.0.1
@author         the-j0k3r (c) 2021
@license        No Redistribution
@preprocessor   stylus
@var number   tab_size      'Tab size '    [8, 2, 8, 1]
==/UserStyle== */

// helpers
i = !important

@-moz-document domain("github.com") {
   // defaults to 8 which is GH default
    pre, .highlight, .diff-table, .tab-size {
       tab-size: tab_size i
       -moz-tab-size: tab_size i
    }
    .cm-tab {
        width: tab_size ch i
    }
    // Hide tab width select from CodeMirror when editing files via browser
    // https://github.com/StylishThemes/GitHub-Dark/pull/959#issuecomment-504646695
    if (tab_size) {
        select[id="indent-size"] {
            display: none i
        }
    }
}

If people dont want to wait for this PR heres something I knocked up for my own GH style to essentially do the same as this PR

should replace the options in GitHub Dark, i.e. dont use those when using this.

@Mottie with ❤️

@ian-h-chamberlain
Copy link

ian-h-chamberlain commented Nov 9, 2021

In case anyone else was going crazy about this, the recommended snippet above didn't seem to work for me in diff mode (width was ~halved from what I expected), but does in other modes. I finally figured out this was a way to fix it for diffs:

/*....
@var number   tab_size      'Tab size '    [8, 2, 8, 1, 'ch']
==/UserStyle== */

I'm not sure why, I guess the default unit has a different size than 1ch for some reason? Anyway, this seems to work for me for now, hopefully it helps if someone else had the same issue.

@the-j0k3r
Copy link
Member

the-j0k3r commented Nov 10, 2021

@ian-h-chamberlain, can you post a link to this page place way to reproduce?

Im interested because you added unit values and that value may have a knock on effect in other places.

Not the least .cm-tab already had that value appended and now it will likely break.

@ian-h-chamberlain
Copy link

@the-j0k3r I noticed after I posted that comment, this is one example (most golang repos use tabs instead of spaces):
https://github.com/golang/go/pull/49359/files

Note, I use the split diff view in the gear dropdown on that page.
In both examples below I have tab_size set to 4 via the Stylus UI.

No units (except .cm-tab as posted originally):
Screen Shot 2021-11-10 at 9 27 45 AM

With ch as the unit, and removed it from .cm-tab to avoid duplicating:
Screen Shot 2021-11-10 at 9 28 06 AM

It still seems slightly off, which is weird since 1ch should be the same as a space in a monospace font, but I'm chalking that up to either the + signs or some other element on the page throwing it off – for me, it's close enough not to matter.

@the-j0k3r
Copy link
Member

With ch as the unit, and removed it from .cm-tab to avoid duplicating:

Well be aware the other not cm-tabs selectors may not work anymore as intended.

I will have a look at it later.

@the-j0k3r
Copy link
Member

Im looking here https://github.com/bivious/k-plus/blob/master/src/modules/dcc/DccFileTransfer.h

and 8ch is always wider than just 8, so no idea.

@the-j0k3r
Copy link
Member

the-j0k3r commented Nov 10, 2021

It still seems slightly off, which is weird since 1ch should be the same as a space in a monospace font, but I'm chalking that up to either the + signs or some other element on the page throwing it off – for me, it's close enough not to matter.

1ch is relative to the width of the "0" (zero), so I wonder if font size will affect that. while the integer is a multiple of the advance width of the space character (U+0020) to be used as the width of tabs. (Character 'SPACE' is 0020) so ch is really not really a reliable if its relative to the width of 0 and depending on the font user has and size will determine the end result.

So need to find an immutable unit that is fixed width no matter the font size.

Else ignore the differences.

@ian-h-chamberlain
Copy link

1ch is relative to the width of the "0" (zero), so I wonder if font size will affect that. while the integer is a multiple of the advance width of the space character (U+0020) to be used as the width of tabs. (Character 'SPACE' is 0020) so ch is really not really a reliable if its relative to the width of 0 and depending on the font user has and size will determine the end result.

Yeah, this is the part that confused me – since I am using a monospace font, I would expect U+0020 and 0 to have the same width, but there is definitely a difference in appearance when using plain integer vs ch.

For posterity, this is the full style I am now using, and it seems to work well enough for my purposes – but maybe others people's preferences would be different.

/* ==UserStyle==
@name           GitHub Tabs
@namespace      Tabs for GitHub
@version        1.0.0
@description    A customizable tab width style for GitHub
@author         the-j0k3r (c) 2021
@license        No Redistribution
@preprocessor   stylus                          
@var            number  tab_size    "Tab size"  [8,      2,   8,   1,    "ch"]
                                                default, min, max, step, units
==/UserStyle== */

@-moz-document domain("github.com") {
    // defaults to 8 which is GH default
    pre, .highlight, .diff-table, .tab-size {
            tab-size: tab_size !important;
       -moz-tab-size: tab_size !important;
    }
    .cm-tab {
        width: tab_size !important;
    }
    // Hide tab width select from CodeMirror when editing files via browser
    // https://github.com/StylishThemes/GitHub-Dark/pull/959#issuecomment-504646695
    if (tab_size) {
        select[id="indent-size"] {
            display: none !important;
        }
    }
}

@the-j0k3r
Copy link
Member

the-j0k3r commented Nov 11, 2021

The font in use doesn't matter, because a 14/16/other px sizes will affect the width of 0 more than the width of space.

Stackoveflows as some things to say about this https://stackoverflow.com/questions/24970502/what-is-the-width-of-empty-space-in-html

It doesn't mention a comparison between width of 0/space, but I'm confident that ch is a better choice in this case.

FYI n my GitHub style I am using ch same way you have altered the style, since I no longer use GHD or the standalone snippet above, however I will experiment with absolute lengths rather than relative see https://www.w3schools.com/CSSref/css_units.asp

@ian-h-chamberlain
Copy link

From a quick experiment, it seems like 0.25em is pretty close to the width of a space (U+0020) character (using https://jkorpela.fi/styles/spaces.html as a reference), but I'm not sure that it's necessarily better than using ch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants