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

Suggest changing lisp-indent-function #1602

Closed
wants to merge 1 commit into from

Conversation

@raxod502
Copy link
Contributor

commented May 27, 2018

See also #1601.

@raxod502 raxod502 referenced this pull request May 27, 2018
CONTRIBUTING.org Outdated
It is provided for you in [[https://github.com/abo-abo/swiper/blob/master/.dir-locals.el][.dir-locals.el]], please obey it.

The setting for =indent-tabs-mode= is automatically applied by
=.dir-locals.el=. In Emacs 26, you can apply the setting for

This comment has been minimized.

Copy link
@basil-conto

basil-conto May 28, 2018

Collaborator

What is the significance of Emacs version 26? Doesn't the suggestion work in prior versions?

Nitpick: please honour the sentence-end-double-space setting in the project's dir-locals-file.

This comment has been minimized.

Copy link
@raxod502

raxod502 May 28, 2018

Author Contributor

.dir-locals-2.el support was added only in Emacs 26. I'll fix the sentence spacing, sorry about that.

This comment has been minimized.

Copy link
@raxod502

raxod502 May 28, 2018

Author Contributor

Wait, on second thought—the rest of the file also uses single spaces after periods! I could fix the rest of it as well, though.

This comment has been minimized.

Copy link
@basil-conto

basil-conto May 28, 2018

Collaborator

.dir-locals-2.el support was added only in Emacs 26

Right, but there's never been something stopping users from modifying .dir-locals.el directly. My point is that the current wording suggests the entire recipe ((add-to-list ...) included) is specific to Emacs 26, when only the .dir-locals-2.el suggestion is.

CONTRIBUTING.org Outdated
#+end_src
It is provided for you in [[https://github.com/abo-abo/swiper/blob/master/.dir-locals.el][.dir-locals.el]], please obey it.

The setting for =indent-tabs-mode= is automatically applied by

This comment has been minimized.

Copy link
@basil-conto

basil-conto May 28, 2018

Collaborator

Shouldn't code be marked up with tildes? I.e. ~indent-tabs-mode~, ~lisp-indent-function~, etc.

This comment has been minimized.

Copy link
@raxod502

raxod502 May 28, 2018

Author Contributor

The rest of the file uses = monospace rather than ~ code. I could change that, too, though.

This comment has been minimized.

Copy link
@basil-conto

basil-conto May 28, 2018

Collaborator

The rest of the file uses = monospace rather than ~ code. I could change that, too, though.

fix-all-things

Silliness aside, I'd like to know whether there's a good reason to use = for everything. If there isn't, then I'd prefer to use ~ where appropriate. This can, of course, be done at a later time, but if we agree on using ~, then we might as well use it for any new additions.

This comment has been minimized.

Copy link
@abo-abo

abo-abo Jun 7, 2018

Owner

Actually, within many of my projects I've made an ad-hoc convention that ~...~ should be used as equivalent of <kbd>...</kbd> in Markdown. With a bit of customization, these tags will be properly exported both in HTML and in Info. So I'd like to keep ~...~ exclusively for keyboard shortcuts.

This comment has been minimized.

Copy link
@basil-conto

basil-conto Jun 7, 2018

Collaborator

Fair enough.

CONTRIBUTING.org Outdated
#+end_src

to your init-file, copying =.dir-locals.el= to =.dir-locals-2.el=, and
uncommenting the line in that file which sets =lisp-indent-function=.

This comment has been minimized.

Copy link
@basil-conto

basil-conto May 28, 2018

Collaborator

If we really are going to go full common-lisp-indent-function (on which decision I've commented in #1601), wouldn't it make more sense to uncomment the correspoding dir-locals-file line?

This comment has been minimized.

Copy link
@raxod502

raxod502 May 28, 2018

Author Contributor

By default, Emacs complains about the unsafe file-local variable, every time you open any file in the repository. That seems overly annoying, although my mind could be changed here.

This comment has been minimized.

Copy link
@basil-conto

basil-conto May 28, 2018

Collaborator

By default, Emacs complains about the unsafe file-local variable, every time you open any file in the repository. That seems overly annoying

Indeed, which is why I thought we were suggesting the safe-local-variable-values spiel. I realise now, though, that "normal" users (i.e. users not hacking on Swiper) deserve to not have to play with safe-local-variable-values in order to quietly read the Swiper sources.

@raxod502 raxod502 force-pushed the raxod502:feat/lisp-indent-function branch to a8ecca6 May 29, 2018

@raxod502

This comment has been minimized.

Copy link
Contributor Author

commented May 29, 2018

I changed the sentences to end with two spaces, replaced = with ~ where appropriate, and ditched the stuff about .dir-locals-2.el.

@abo-abo abo-abo closed this in f17d51d Jun 7, 2018

abo-abo added a commit that referenced this pull request Jun 7, 2018
@abo-abo

This comment has been minimized.

Copy link
Owner

commented Jun 7, 2018

Thanks.

@raxod502 raxod502 deleted the raxod502:feat/lisp-indent-function branch Jun 8, 2018

basil-conto added a commit to basil-conto/swiper that referenced this pull request Aug 6, 2018
Standardise defalias indentation
CONTRIBUTING.org: Suggest common-lisp-indent-function setting
similar to that of lisp-indent-function.
counsel.el (counsel--push-xref-marker):
ivy.el (ivy--dirname-p): Reindent accordingly.

Re: abo-abo#1601, abo-abo#1602, abo-abo#1612
abo-abo added a commit that referenced this pull request Aug 6, 2018
Standardise defalias indentation
CONTRIBUTING.org: Suggest common-lisp-indent-function setting
similar to that of lisp-indent-function.
counsel.el (counsel--push-xref-marker):
ivy.el (ivy--dirname-p): Reindent accordingly.

Re: #1601, #1602, #1612
Fixes #1702
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.