updated emacs configuration

On Thu, Jun 13, 2013 at 6:27 PM, Peter Eisentraut <peter_e@gmx.net> wrote:

First, I propose adding a .dir-locals.el file to the top-level directory
with basic emacs settings. These get applied automatically. This
especially covers the particular tab and indentation settings that
PostgreSQL uses. With this, casual developers will not need to modify
any of their emacs settings.

Yes please. I've had the pgsql stuff in my .emacs for-ever (ever since I
was a student and compelled to do homework on Postgres) and knew the
magical rules about naming the directory, but it always felt so dirty
and very much a 'you need to know the trick' level of intimacy.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Hi,

Peter Eisentraut <peter_e@gmx.net> writes:

I think the suggested emacs configuration snippets in
src/tools/editors/emacs.samples no longer represent current best
practices. I have come up with some newer things that I'd like to
propose for review.

Thanks for doing that!

First, I propose adding a .dir-locals.el file to the top-level directory
with basic emacs settings. These get applied automatically. This
especially covers the particular tab and indentation settings that
PostgreSQL uses. With this, casual developers will not need to modify
any of their emacs settings.

I've tested that on a new git clone and with the `emacs -q` command so
as not to load any of my local setup. While the indentation seemed ok,
the placement of the comments seems way off:

Compare what you see using those commands:

emacs -q src/backend/commands/extension.c
emacs -q -l ../emacs.samples src/backend/commands/extension.c

(When using macosx, you might have to replace the 'emacs' binary
location with /Applications/Emacs.app/Contents/MacOS/Emacs).

I did also test on doc/src/sgml/extend.sgml and some Makefile, only with
using the emacs.samples file content though.

With that, emacs.samples can be shrunk significantly. The only real
reason to keep is that that c-offsets-alist and (more dubiously)
sgml-basic-offset cannot be set from .dir-locals.el because they are not
"safe". I have also removed many of the redundant examples and settled
on a hook-based solution.

A couple of notes about your emacs.sample file:

- Name the lambda used in the hook for easier removing / reference

- A fresh git clone will create a directory named postgres, so I did
change your /postgresql/ regex to /postgres/ in my attached version

I think together this setup would be significantly simpler and more
practical.

Agreed.
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support

On 06/13/2013 09:27 PM, Peter Eisentraut wrote:

I think the suggested emacs configuration snippets in
src/tools/editors/emacs.samples no longer represent current best
practices. I have come up with some newer things that I'd like to
propose for review.

First, I propose adding a .dir-locals.el file to the top-level directory
with basic emacs settings. These get applied automatically. This
especially covers the particular tab and indentation settings that
PostgreSQL uses. With this, casual developers will not need to modify
any of their emacs settings.

(In the attachment, .dir-locals.el is called _dir-locals.el so that it
doesn't get lost. To clarify, it goes into the same directory that
contains configure.in.)

With that, emacs.samples can be shrunk significantly. The only real
reason to keep is that that c-offsets-alist and (more dubiously)
sgml-basic-offset cannot be set from .dir-locals.el because they are not
"safe". I have also removed many of the redundant examples and settled
on a hook-based solution.

I think together this setup would be significantly simpler and more
practical.

The idea is a very good one in principle, but my short experiment with
the provided .dir-locals.el didn't give me BSD style brace indentation.
It works if we can do those "unsafe" things, but then we surely don't
want to have a user prompted to accept the settings each time. If
.dir-locals.el won't do what we need on its own, it seems to me hardly
worth having.

cheers

andrew

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Thu, Jun 13, 2013 at 09:27:07PM -0400, Peter Eisentraut wrote:

I think the suggested emacs configuration snippets in
src/tools/editors/emacs.samples no longer represent current best
practices. I have come up with some newer things that I'd like to
propose for review.

((c-mode . ((c-basic-offset . 4)
(fill-column . 79)

I don't know whether you'd consider it to fall within the scope of this
update, but 78 is the fill-column setting that matches pgindent.

(sgml-mode . ((fill-column . 79)
(indent-tabs-mode . nil))))

SGML fill-column practice has varied widely. I estimate 72 is more the norm,
though I personally favor longer lines like we use in source code comments.

--
Noah Misch
EnterpriseDB http://www.enterprisedb.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 06/23/2013 03:43 PM, Andrew Dunstan wrote:

The idea is a very good one in principle, but my short experiment with
the provided .dir-locals.el didn't give me BSD style brace
indentation. It works if we can do those "unsafe" things, but then we
surely don't want to have a user prompted to accept the settings each
time. If .dir-locals.el won't do what we need on its own, it seems to
me hardly worth having.

However, it does work if you add this at the beginning of the c-mode list:

(c-file-style . "bsd")

cheers

andrew

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Andrew Dunstan <andrew@dunslane.net> writes:

The idea is a very good one in principle, but my short experiment with
the provided .dir-locals.el didn't give me BSD style brace indentation.
It works if we can do those "unsafe" things, but then we surely don't
want to have a user prompted to accept the settings each time. If
.dir-locals.el won't do what we need on its own, it seems to me hardly
worth having.

I'm un-thrilled with this as well, though for a slightly different
reason: we have a policy that the PG sources should be tool agnostic,
and in fact removed file-local emacs settings awhile back because of
that. Even though I use emacs, I would much rather keep such settings
in my personal library. (TBH, I keep enable-local-variables turned off,
and would thus get no benefit from the proposed file anyhow.)

Another thing I'm not too happy about is that the proposed patch
summarily discards the information we had about how to work with older
emacs versions. I'm not sure it will actually break anybody's setup,
but that would depend on whether .dir-locals.el was added before or
after the last round of rewriting of c-mode.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Fri, 2013-06-21 at 11:20 +0200, Dimitri Fontaine wrote:

I've tested that on a new git clone and with the `emacs -q` command so
as not to load any of my local setup. While the indentation seemed ok,
the placement of the comments seems way off:

Compare what you see using those commands:

emacs -q src/backend/commands/extension.c
emacs -q -l ../emacs.samples src/backend/commands/extension.c

Well, the first one uses 8-space tabs, the second 4-space tabs, so they
look completely different. I'm not sure what you are trying to point
out.

A couple of notes about your emacs.sample file:

- Name the lambda used in the hook for easier removing / reference

Interesting, I had never thought of that. I don't see that used in
Emacs source code or core packages, however. Do you have a reference
that this is recommended practice?

- A fresh git clone will create a directory named postgres, so I did
change your /postgresql/ regex to /postgres/ in my attached version

Huh?

$ git clone git://git.postgresql.org/git/postgresql.git
Cloning into 'postgresql'...

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Sun, 2013-06-23 at 16:11 -0400, Andrew Dunstan wrote:

The idea is a very good one in principle, but my short experiment with
the provided .dir-locals.el didn't give me BSD style brace
indentation. It works if we can do those "unsafe" things, but then we
surely don't want to have a user prompted to accept the settings each
time. If .dir-locals.el won't do what we need on its own, it seems to
me hardly worth having.

The main win will be to get 4-space tabs for casual observers, so the
files don't look crappy to newbies.

However, it does work if you add this at the beginning of the c-mode list:

(c-file-style . "bsd")

You are right, that should be added.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Sun, 2013-06-23 at 16:03 -0400, Noah Misch wrote:

((c-mode . ((c-basic-offset . 4)
(fill-column . 79)

I don't know whether you'd consider it to fall within the scope of this
update, but 78 is the fill-column setting that matches pgindent.

Well, well, well. I did some extensive tests some time ago when that
setting was added. I have a suspicion that this could be related to the
recent pgindent changes (which everyone claims were no changes). I'm
going to have to research that some more.

(sgml-mode . ((fill-column . 79)
(indent-tabs-mode . nil))))

SGML fill-column practice has varied widely. I estimate 72 is more the norm,
though I personally favor longer lines like we use in source code comments.

I have no strong feelings about this.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Sun, 2013-06-23 at 16:37 -0400, Tom Lane wrote:

I'm un-thrilled with this as well, though for a slightly different
reason: we have a policy that the PG sources should be tool agnostic,
and in fact removed file-local emacs settings awhile back because of
that.

We don't want to keep the tool settings in the sources, but what's wrong
with keeping them next to the sources? We have
src/tools/editor/emacs.samples already. This just moves some of the
contents to .dir-locals.el, where it would be more useful.

Even though I use emacs, I would much rather keep such settings
in my personal library. (TBH, I keep enable-local-variables turned
off, and would thus get no benefit from the proposed file anyhow.)

No one is required to use this, and seasoned hackers aren't exactly the
target audience anyway.

Another thing I'm not too happy about is that the proposed patch
summarily discards the information we had about how to work with older
emacs versions. I'm not sure it will actually break anybody's setup,
but that would depend on whether .dir-locals.el was added before or
after the last round of rewriting of c-mode.

I think you are confusing two separate changes. The proposed addition
of .dir-locals.el is entirely optional. (Well, I have removed the Perl
settings from emacs.samples. We can keep them if disabling local
variables is a wide-spread practice.) The C mode settings in the new
emacs.samples file are just as functional as the old ones, except they
are arguably better.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Peter Eisentraut <peter_e@gmx.net> writes:

emacs -q src/backend/commands/extension.c
emacs -q -l ../emacs.samples src/backend/commands/extension.c

Well, the first one uses 8-space tabs, the second 4-space tabs, so they
look completely different. I'm not sure what you are trying to point
out.

With the .dir-locals.el in place, the first one should be using 4-space
tabs too, right?

- Name the lambda used in the hook for easier removing / reference

Interesting, I had never thought of that. I don't see that used in
Emacs source code or core packages, however. Do you have a reference
that this is recommended practice?

Well a friend of mine pointed that out to me one day, and he's an Emacs
and Gnus commiter. The thing is that you're not guaranteed that the same
lambda source code will evaluate to the same object each time, and that
would prevent you to remove-hook.

$ git clone git://git.postgresql.org/git/postgresql.git
Cloning into 'postgresql'...

I can reproduce that here. I don't know why I have those postgres dirs
then, and I'm pretty confused about my round of testing now.

--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 26/06/13 10:51, Dimitri Fontaine wrote:

Peter Eisentraut <peter_e@gmx.net> writes:

$ git clone git://git.postgresql.org/git/postgresql.git
Cloning into 'postgresql'...

I can reproduce that here. I don't know why I have those postgres dirs
then, and I'm pretty confused about my round of testing now.

Maybe you cloned from GitHub, where the mirrored repository is called
'postgres'?

Cheers,
Jan

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Tue, Jun 25, 2013 at 11:17:47PM -0400, Peter Eisentraut wrote:

On Sun, 2013-06-23 at 16:03 -0400, Noah Misch wrote:

((c-mode . ((c-basic-offset . 4)
(fill-column . 79)

I don't know whether you'd consider it to fall within the scope of this
update, but 78 is the fill-column setting that matches pgindent.

Well, well, well. I did some extensive tests some time ago when that
setting was added. I have a suspicion that this could be related to the
recent pgindent changes (which everyone claims were no changes). I'm
going to have to research that some more.

pgindent has not changed the following xlog.c comment since April 2011, but
fill-column 77 or 79 changes it:

/*
* fullPageWrites is the master copy used by all backends to determine
* whether to write full-page to WAL, instead of using process-local one.
* This is required because, when full_page_writes is changed by SIGHUP,
* we must WAL-log it before it actually affects WAL-logging by backends.
* Checkpointer sets at startup or after SIGHUP.
*/

Note that emacs and pgindent remain at odds over interior tabs in comments.
When pgindent finds a double-space (typically after a sentence) ending at a
tab stop, it replaces the double-space with a tab. c-fill-paragraph will
convert that tab to a *single* space, and that can be enough to change many
line break positions.

--
Noah Misch
EnterpriseDB http://www.enterprisedb.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Noah Misch wrote:

Note that emacs and pgindent remain at odds over interior tabs in comments.
When pgindent finds a double-space (typically after a sentence) ending at a
tab stop, it replaces the double-space with a tab. c-fill-paragraph will
convert that tab to a *single* space, and that can be enough to change many
line break positions.

We should really stop pgindent from converting those double-spaces to
tabs. Those tabs are later changed to three or four spaces when wording
of the comment is changed, and things start looking very odd.

Really, we should get out of patched BSD indent entirely already. How
about astyle, for instance? I'm told that with some sensible options,
the diff to what we have now is not very large, and several things
actually become sensible (such as function pointer decls, which are
messed up rather badly by pgindent)

--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Thu, Jun 27, 2013 at 03:51:15PM -0400, Alvaro Herrera wrote:

Noah Misch wrote:

Note that emacs and pgindent remain at odds over interior tabs in comments.
When pgindent finds a double-space (typically after a sentence) ending at a
tab stop, it replaces the double-space with a tab. c-fill-paragraph will
convert that tab to a *single* space, and that can be enough to change many
line break positions.

We should really stop pgindent from converting those double-spaces to
tabs. Those tabs are later changed to three or four spaces when wording
of the comment is changed, and things start looking very odd.

Really, we should get out of patched BSD indent entirely already. How
about astyle, for instance? I'm told that with some sensible options,
the diff to what we have now is not very large, and several things
actually become sensible (such as function pointer decls, which are
messed up rather badly by pgindent)

Sounds good to me.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

Noah Misch wrote:

Note that emacs and pgindent remain at odds over interior tabs in comments.
When pgindent finds a double-space (typically after a sentence) ending at a
tab stop, it replaces the double-space with a tab. c-fill-paragraph will
convert that tab to a *single* space, and that can be enough to change many
line break positions.

We should really stop pgindent from converting those double-spaces to
tabs. Those tabs are later changed to three or four spaces when wording
of the comment is changed, and things start looking very odd.

+1. That's probably the single most annoying bit of behavior in pgindent.
Being a two-spaces-after-a-period kind of guy, it might bite me more
often than other people, but now that somebody else has brought it up...

Really, we should get out of patched BSD indent entirely already. How
about astyle, for instance? I'm told that with some sensible options,
the diff to what we have now is not very large, and several things
actually become sensible (such as function pointer decls, which are
messed up rather badly by pgindent)

AFAIR, no one has ever done a serious comparison to anything except GNU
indent, and (at least at the time) it seemed to have bugs as bad as
pgindent's, just different ones. I'm certainly open to another choice
as long as we don't lose on portability of the tool. But who will do
the legwork to test something else?

Probably the principal argument against switching to a different tool
has been that whitespace changes would complicate back-patching, but
I don't see why we couldn't solve that by re-indenting all the live back
branches at the time of the switch.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 2013-06-27 17:31:54 -0400, Tom Lane wrote:

Really, we should get out of patched BSD indent entirely already. How
about astyle, for instance? I'm told that with some sensible options,
the diff to what we have now is not very large, and several things
actually become sensible (such as function pointer decls, which are
messed up rather badly by pgindent)

AFAIR, no one has ever done a serious comparison to anything except GNU
indent, and (at least at the time) it seemed to have bugs as bad as
pgindent's, just different ones. I'm certainly open to another choice
as long as we don't lose on portability of the tool. But who will do
the legwork to test something else?

I think before doing any serious testing we would need to lay out how
many changes and what changes in formatting we would allow and what kind
of enforced formatting rules we think are required.

Being at least one of the persons having mentioned astyle to Alvaro, I
had tested that once and I thought the results were resembling something
reasonable after an hour of fiddling or so. But there were certain
things that I could not be make it do during that. The only thing I
remember now was reducing the indentation of parameters to the left if
the line length got to long. Now, I personally think that's an
anti-feature, but I am not sure if others think differently.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Andres Freund <andres@2ndquadrant.com> writes:

I think before doing any serious testing we would need to lay out how
many changes and what changes in formatting we would allow and what kind
of enforced formatting rules we think are required.

Well, we certainly never applied any such analysis to pgindent. I
personally don't mind leaving corner cases to the judgment of the tool
author ... of course, what's a corner case and what's important may be
in the eye of the beholder. But it's surely a bug, for instance, that
pgindent is so clueless about function pointers.

Being at least one of the persons having mentioned astyle to Alvaro, I
had tested that once and I thought the results were resembling something
reasonable after an hour of fiddling or so. But there were certain
things that I could not be make it do during that. The only thing I
remember now was reducing the indentation of parameters to the left if
the line length got to long. Now, I personally think that's an
anti-feature, but I am not sure if others think differently.

I never particularly cared for that behavior either. It probably made
sense back in the video-terminal days, when your view of a program was
80 columns period. These days I think most people can use a wider
window at need --- not that I want to adopt wider lines as standard, but
the readability tradeoff between not having lines wrap versus messing up
the indentation seems like it's probably different now.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Tom Lane wrote:

Andres Freund <andres@2ndquadrant.com> writes:

Being at least one of the persons having mentioned astyle to Alvaro, I
had tested that once and I thought the results were resembling something
reasonable after an hour of fiddling or so. But there were certain
things that I could not be make it do during that. The only thing I
remember now was reducing the indentation of parameters to the left if
the line length got to long. Now, I personally think that's an
anti-feature, but I am not sure if others think differently.

I never particularly cared for that behavior either. It probably made
sense back in the video-terminal days, when your view of a program was
80 columns period.

I've never liked that either; I am a fan of keeping things to 80
columns, but when things get longer I prefer my editor to wrap them to
the next line without the silly de-indent (or not wrap, if I tell it not
to.)

Another benefit of more modern tools is that there's no need for a
typedef file, which is great when you're trying to indent after a patch
which adds some more typedefs that are not listed in the file.

These days I think most people can use a wider
window at need --- not that I want to adopt wider lines as standard, but
the readability tradeoff between not having lines wrap versus messing up
the indentation seems like it's probably different now.

Agreed. I would be sad if we adopted a policy of sloppiness on width.

--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers