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

EDITORIAL: Update emacs & vim modeline. #6096

Closed

Conversation

ArthurSonzogni
Copy link
Member

@ArthurSonzogni ArthurSonzogni commented Oct 22, 2020

  1. Move emacs & vim modeline to the end of the file. This is not an
    important part of the spec, so it doesn't deserve being put at the
    front.

  2. Set the vim filetype to be html. Depending on the developers
    configuration, this usually brings the HTML syntax color.

  3. Set the vim color-column to be 100. This is for consistency
    with emacs's modeline.


/acknowledgements.html ( diff )

1. Move emacs & vim modeline to the end of the file. This is not an
   important part of the spec, so it doesn't deserve being put at the
   front.

2. Set the vim filetype to be html. Depending on the developers
   configuration, this usually brings the HTML syntax color.

3. Set the vim column-column to be 100. This is for being consistent
   with the emacs's modeline.
@annevk
Copy link
Member

annevk commented Oct 22, 2020

Is this redundant with .editorconfig by chance? I'd rather remove it completely. Don't think our other standards have this.

@ArthurSonzogni
Copy link
Member Author

Thanks for your reply!

Is this redundant with .editorconfig by chance? I'd rather remove it completely. Don't think our other standards have this.

I wasn't aware of the existence of https://editorconfig.org/
This is a nice initiative, but not working by default (this requires installing a plugin) and is not easily discoverable if you don't know what this is.


I will let you choose what you prefer in between keeping or not this modelines. My patch was about moving/updating existing modeslines. It started to become painful entering vim commands to get colors ;-)

@annevk annevk requested a review from domenic October 22, 2020 14:46
@sideshowbarker
Copy link
Contributor

Is this redundant with .editorconfig by chance? I'd rather remove it completely. Don't think our other standards have this.

Instead of removing the modeline, I’d personally rather we added it to our other standards.

The benefit of the modeline is that it’s one single line that’s extremely simple and minimal (by design) yet has the effect of making the most important thing we want work in both vim and emacs right out of the box — without any vim or emacs users needing to do any special configuration to make it happen — whereas enabling support for .editorconfig requires vim and emacs users to do additional configuration to enable it. And that’s something I suspect many or most vim and emacs users don’t bother to do. (I’m a vim user and I’ve never done anything anywhere ever to enable support for .editorconfig.)

All that said, I would rather we removed the modeline completely rather than adding to it further. So I don’t support merging the change in this PR (which anyway are changes that’ll only have effect in vim but not in emacs).

@ArthurSonzogni
Copy link
Member Author

It's up to you. Should I abandon this PR?

@annevk
Copy link
Member

annevk commented Oct 26, 2020

@domenic should decide.

@domenic
Copy link
Member

domenic commented Oct 26, 2020

I tend to agree that, if anything, we should delete these. Giving special treatment to vim/emacs, while all other text editors standardize on using editorconfig, seems wrong. Let's close this PR. Sorry @ArthurSonzogni!

@domenic domenic closed this Oct 26, 2020
@ArthurSonzogni ArthurSonzogni deleted the editorial/modeline branch October 27, 2020 09:04
@ArthurSonzogni
Copy link
Member Author

Sorry @ArthurSonzogni! ~@domenic

Thanks for taking a decision. No worries, I was not very attached to it.

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

Successfully merging this pull request may close these issues.

None yet

4 participants