unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* [PATCH] lib/database.cc: fix misleading indentation
@ 2016-09-27 15:06 Tomi Ollila
  2016-09-28 11:20 ` David Bremner
  2016-10-20 12:54 ` Franz Fellner
  0 siblings, 2 replies; 9+ messages in thread
From: Tomi Ollila @ 2016-09-27 15:06 UTC (permalink / raw)
  To: notmuch; +Cc: tomi.ollila

Found by gcc 6.1.1 -Wmisleading-indentation option (set by -Wall).
---

I suggest this change to be included in 0.23: this does not affect
binary content and this will give users of gcc 6 more enjoyable
compilation experience.

 lib/database.cc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index 392e8b2..4bfae01 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -1685,8 +1685,8 @@ notmuch_database_begin_atomic (notmuch_database_t *notmuch)
 	notmuch->atomic_nesting > 0)
 	goto DONE;
 
-	if (notmuch_database_needs_upgrade(notmuch))
-		return NOTMUCH_STATUS_UPGRADE_REQUIRED;
+    if (notmuch_database_needs_upgrade (notmuch))
+	return NOTMUCH_STATUS_UPGRADE_REQUIRED;
 
     try {
 	(static_cast <Xapian::WritableDatabase *> (notmuch->xapian_db))->begin_transaction (false);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] lib/database.cc: fix misleading indentation
  2016-09-27 15:06 [PATCH] lib/database.cc: fix misleading indentation Tomi Ollila
@ 2016-09-28 11:20 ` David Bremner
  2016-10-20 12:54 ` Franz Fellner
  1 sibling, 0 replies; 9+ messages in thread
From: David Bremner @ 2016-09-28 11:20 UTC (permalink / raw)
  To: Tomi Ollila, notmuch; +Cc: tomi.ollila

Tomi Ollila <tomi.ollila@iki.fi> writes:

> Found by gcc 6.1.1 -Wmisleading-indentation option (set by -Wall).
> ---

applied to release and master

d

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] lib/database.cc: fix misleading indentation
  2016-09-27 15:06 [PATCH] lib/database.cc: fix misleading indentation Tomi Ollila
  2016-09-28 11:20 ` David Bremner
@ 2016-10-20 12:54 ` Franz Fellner
  2016-10-20 16:47   ` David Bremner
  1 sibling, 1 reply; 9+ messages in thread
From: Franz Fellner @ 2016-10-20 12:54 UTC (permalink / raw)
  To: Tomi Ollila, notmuch; +Cc: tomi.ollila

Probably a little bit off-topic, but somewhat related.
There is a wild mix of TABs and spaces used for indenting all over the place.
I had a hard time to follow the code. Could there be done something?
Do we have some sort of coding style? If not could we agree on some? At least
for such important things as indentation... It is possible to use e.g. astyle
to apply the formatting automatically.

Franz

On Tue, 27 Sep 2016 18:06:52 +0300, Tomi Ollila <tomi.ollila@iki.fi> wrote:
> Found by gcc 6.1.1 -Wmisleading-indentation option (set by -Wall).
> ---
> 
> I suggest this change to be included in 0.23: this does not affect
> binary content and this will give users of gcc 6 more enjoyable
> compilation experience.
> 
>  lib/database.cc | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/database.cc b/lib/database.cc
> index 392e8b2..4bfae01 100644
> --- a/lib/database.cc
> +++ b/lib/database.cc
> @@ -1685,8 +1685,8 @@ notmuch_database_begin_atomic (notmuch_database_t *notmuch)
>         notmuch->atomic_nesting > 0)
>         goto DONE;
>  
> -       if (notmuch_database_needs_upgrade(notmuch))
> -               return NOTMUCH_STATUS_UPGRADE_REQUIRED;
> +    if (notmuch_database_needs_upgrade (notmuch))
> +       return NOTMUCH_STATUS_UPGRADE_REQUIRED;
>  
>      try {
>         (static_cast <Xapian::WritableDatabase *> (notmuch->xapian_db))->begin_transaction (false);
> -- 
> 2.7.4
> 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> https://notmuchmail.org/mailman/listinfo/notmuch
> 
> 
-- 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] lib/database.cc: fix misleading indentation
  2016-10-20 12:54 ` Franz Fellner
@ 2016-10-20 16:47   ` David Bremner
  2016-10-20 18:30     ` Franz Fellner
  0 siblings, 1 reply; 9+ messages in thread
From: David Bremner @ 2016-10-20 16:47 UTC (permalink / raw)
  To: Franz Fellner, Tomi Ollila, notmuch; +Cc: tomi.ollila

Franz Fellner <alpine.art.de@gmail.com> writes:

> Probably a little bit off-topic, but somewhat related.
> There is a wild mix of TABs and spaces used for indenting all over the place.
> I had a hard time to follow the code. Could there be done something?
> Do we have some sort of coding style? If not could we agree on some? At least
> for such important things as indentation... It is possible to use e.g. astyle
> to apply the formatting automatically.
>
> Franz

from devel/STYLE:

* Indent is 4 spaces with mixed tab/spaces and a tab width of 8.
  (Specifically, a line should begin with zero or more tabs followed
  by fewer than eight spaces.)

that _should_ be enforced by devel/uncrustify.cfg

On the other hand, there are also no doubt places in the code where this
rule is not adhered to. At the risk of being clichéd, whitespace fixup
patches are welcome.

Finally, it might help to have some settings whatever editor you are
using. I'm happy add a file to add a project .vimrc [1] (or whatever editor
people are using) to complement the .dir-locals.el that is there.

d

[1]: all I know about that I read on https://andrew.stwrt.ca/posts/project-specific-vimrc/

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] lib/database.cc: fix misleading indentation
  2016-10-20 16:47   ` David Bremner
@ 2016-10-20 18:30     ` Franz Fellner
  2016-10-20 23:45       ` David Bremner
  2016-10-21 13:54       ` editorconfig David Bremner
  0 siblings, 2 replies; 9+ messages in thread
From: Franz Fellner @ 2016-10-20 18:30 UTC (permalink / raw)
  To: David Bremner, Tomi Ollila, notmuch; +Cc: tomi.ollila

On Thu, 20 Oct 2016 13:47:40 -0300, David Bremner <david@tethera.net> wrote:
> Franz Fellner <alpine.art.de@gmail.com> writes:
> 
> > Probably a little bit off-topic, but somewhat related.
> > There is a wild mix of TABs and spaces used for indenting all over the place.
> > I had a hard time to follow the code. Could there be done something?
> > Do we have some sort of coding style? If not could we agree on some? At least
> > for such important things as indentation... It is possible to use e.g. astyle
> > to apply the formatting automatically.
> >
> > Franz
> 
> from devel/STYLE:
> 
> * Indent is 4 spaces with mixed tab/spaces and a tab width of 8.
>   (Specifically, a line should begin with zero or more tabs followed
>   by fewer than eight spaces.)

From a quick visual scan at least half the indented lines start with 4 spaces
instead of TABS. As I understand the quoted paragraph that should be wrong.

> that _should_ be enforced by devel/uncrustify.cfg

When was uncrustify used the last time? I simply ran it on lib/database.cc and
it produced a diff with 661 lines - the leading spaces were NOT fixed ;)

> On the other hand, there are also no doubt places in the code where this
> rule is not adhered to. At the risk of being clichéd, whitespace fixup
> patches are welcome.

Well, I could do it but I won't do it by hand ;) If we can fix the uncrustify.cfg
I would be happy to submit patches.

> Finally, it might help to have some settings whatever editor you are
> using. I'm happy add a file to add a project .vimrc [1] (or whatever editor
> people are using) to complement the .dir-locals.el that is there.

Instead of using special config files for every editor we could also stick to
editorconfig:
http://editorconfig.org/

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] lib/database.cc: fix misleading indentation
  2016-10-20 18:30     ` Franz Fellner
@ 2016-10-20 23:45       ` David Bremner
  2016-10-21  6:42         ` Franz Fellner
  2016-10-21 13:54       ` editorconfig David Bremner
  1 sibling, 1 reply; 9+ messages in thread
From: David Bremner @ 2016-10-20 23:45 UTC (permalink / raw)
  To: Franz Fellner, Tomi Ollila, notmuch; +Cc: tomi.ollila

Franz Fellner <alpine.art.de@gmail.com> writes:

> From a quick visual scan at least half the indented lines start with 4 spaces
> instead of TABS. As I understand the quoted paragraph that should be wrong.
>

Can you be specific what what lines in what file? I have the feeling
you're misinterpreting something, or looking at a different file.

>> that _should_ be enforced by devel/uncrustify.cfg
>
> When was uncrustify used the last time? I simply ran it on lib/database.cc and
> it produced a diff with 661 lines - the leading spaces were NOT fixed ;)
>

No idea if it was ever done globally. But I'm also not sure which
leading spaces you are talking about.

d

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] lib/database.cc: fix misleading indentation
  2016-10-20 23:45       ` David Bremner
@ 2016-10-21  6:42         ` Franz Fellner
  2016-10-21  7:01           ` Mark Walters
  0 siblings, 1 reply; 9+ messages in thread
From: Franz Fellner @ 2016-10-21  6:42 UTC (permalink / raw)
  To: David Bremner, Tomi Ollila, notmuch; +Cc: tomi.ollila

On Thu, 20 Oct 2016 20:45:00 -0300, David Bremner <david@tethera.net> wrote:
> Franz Fellner <alpine.art.de@gmail.com> writes:
> 
> > From a quick visual scan at least half the indented lines start with 4 spaces
> > instead of TABS. As I understand the quoted paragraph that should be wrong.
> >
> 
> Can you be specific what what lines in what file? I have the feeling
> you're misinterpreting something, or looking at a different file.

This is from lib/database.cc:

const char *
_find_prefix (const char *name)
{
    unsigned int i;

    for (i = 0; i < ARRAY_SIZE (BOOLEAN_PREFIX_INTERNAL); i++) {
	if (strcmp (name, BOOLEAN_PREFIX_INTERNAL[i].name) == 0)
	    return BOOLEAN_PREFIX_INTERNAL[i].prefix;
    }

    for (i = 0; i < ARRAY_SIZE (BOOLEAN_PREFIX_EXTERNAL); i++) {
	if (strcmp (name, BOOLEAN_PREFIX_EXTERNAL[i].name) == 0)
	    return BOOLEAN_PREFIX_EXTERNAL[i].prefix;
    }

    for (i = 0; i < ARRAY_SIZE (PROBABILISTIC_PREFIX); i++) {
	if (strcmp (name, PROBABILISTIC_PREFIX[i].name) == 0)
	    return PROBABILISTIC_PREFIX[i].prefix;
    }

    INTERNAL_ERROR ("No prefix exists for '%s'\n", name);

    return "";
}

I set ":set list" and ":set listchars=tab:>-" to visualize tabs in vim. This shows me
* the first two codelines (leaving out empty lines) are indented by 4 spaces
* the first line in the first for loop is indented by only one TAB (IMHO should be
  two, because it is one leven down): this is a pattern you can find throughout the
  codebase.
* the next two lines are indented by four spaces
* the first line of the second for-loop is indented by one TAB.
* and so on...

Am I misinterpreting something here? Is this intended to be done that way?
I also get confused by the different lengths of TAB/space-indenting. But probably
that's because I never used it...

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] lib/database.cc: fix misleading indentation
  2016-10-21  6:42         ` Franz Fellner
@ 2016-10-21  7:01           ` Mark Walters
  0 siblings, 0 replies; 9+ messages in thread
From: Mark Walters @ 2016-10-21  7:01 UTC (permalink / raw)
  To: Franz Fellner, David Bremner, Tomi Ollila, notmuch; +Cc: tomi.ollila


Hi

> * the first line in the first for loop is indented by only one TAB (IMHO should be
>   two, because it is one leven down): this is a pattern you can find throughout the
>   codebase.

I think  this is the key confusion -- the notmuch convention is that 
1 tab = 8 spaces

The only other rule is that you should prefer tabs to spaces so you
should never have more than 7 spaces -- for example an indentation of 12
would be 1 tab + 4 spaces.

Best wishes

Mark



> * the next two lines are indented by four spaces
> * the first line of the second for-loop is indented by one TAB.
> * and so on...
>
> Am I misinterpreting something here? Is this intended to be done that way?
> I also get confused by the different lengths of TAB/space-indenting. But probably
> that's because I never used it...
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> https://notmuchmail.org/mailman/listinfo/notmuch

^ permalink raw reply	[flat|nested] 9+ messages in thread

* editorconfig
  2016-10-20 18:30     ` Franz Fellner
  2016-10-20 23:45       ` David Bremner
@ 2016-10-21 13:54       ` David Bremner
  1 sibling, 0 replies; 9+ messages in thread
From: David Bremner @ 2016-10-21 13:54 UTC (permalink / raw)
  To: Franz Fellner, notmuch

Franz Fellner <alpine.art.de@gmail.com> writes:

> Instead of using special config files for every editor we could also
> stick to editorconfig: http://editorconfig.org/

I guess I'm working from the assumption that the majority of people
actively working on notmuch are using emacs or vim. Since it requires
both vim and emacs users to download and configure some plugin I'm
not sure editorconfig is really a win.

Maybe we could start by someone creating a config file for this and
putting it on the wiki.  If it seems useful and maintained, then I'm
happy to ship it.

d

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2016-10-21 13:54 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-09-27 15:06 [PATCH] lib/database.cc: fix misleading indentation Tomi Ollila
2016-09-28 11:20 ` David Bremner
2016-10-20 12:54 ` Franz Fellner
2016-10-20 16:47   ` David Bremner
2016-10-20 18:30     ` Franz Fellner
2016-10-20 23:45       ` David Bremner
2016-10-21  6:42         ` Franz Fellner
2016-10-21  7:01           ` Mark Walters
2016-10-21 13:54       ` editorconfig David Bremner

Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).