[PATCH] lib/database.cc: fix misleading indentation

[PATCH] lib/database.cc: fix misleading indentation

[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

Re: [PATCH] lib/database.cc: fix misleading indentation

[David Bremner]

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

Re: [PATCH] lib/database.cc: fix misleading indentation

[Franz Fellner]

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
> 
> 
-- 

Re: [PATCH] lib/database.cc: fix misleading indentation

[David Bremner]

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/

Re: [PATCH] lib/database.cc: fix misleading indentation

[Franz Fellner]

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/

Re: [PATCH] lib/database.cc: fix misleading indentation

[David Bremner]

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

Re: [PATCH] lib/database.cc: fix misleading indentation

[Franz Fellner]

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...

Re: [PATCH] lib/database.cc: fix misleading indentation

[Mark Walters]

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

editorconfig

[David Bremner]

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