[Qemu-devel] [PATCH] .dir-locals.el: Explicitly set indentation level

Thiago Jung Bauermann posted 1 patch 6 years, 8 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20170818174614.10930-1-bauerman@linux.vnet.ibm.com
Test FreeBSD passed
Test checkpatch passed
Test docker passed
Test s390x passed
.dir-locals.el | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
[Qemu-devel] [PATCH] .dir-locals.el: Explicitly set indentation level
Posted by Thiago Jung Bauermann 6 years, 8 months ago
At least in some configurations, setting c-file-style is not enough to
conform to the QEMU coding style, so explicitly set c-basic-offset as well.

Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
---

My emacs was using indentation level of 8 spaces and this patch convinced
it to use the correct value.

I set c-basic-offset set to tab-width in my ~/.spacemacs which perhaps
isn't the wisest thing to do, but since there's a .dir-locals.el we can
make the editor always do the right thing.

 .dir-locals.el | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/.dir-locals.el b/.dir-locals.el
index 3ac0cfc6f0d6..8e47418c5996 100644
--- a/.dir-locals.el
+++ b/.dir-locals.el
@@ -1,2 +1,3 @@
 ((c-mode . ((c-file-style . "stroustrup")
-	    (indent-tabs-mode . nil))))
+	    (indent-tabs-mode . nil)
+	    (c-basic-offset . 4))))


Re: [Qemu-devel] [PATCH] .dir-locals.el: Explicitly set indentation level
Posted by Thiago Jung Bauermann 6 years, 7 months ago
Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com> writes:

> At least in some configurations, setting c-file-style is not enough to
> conform to the QEMU coding style, so explicitly set c-basic-offset as well.
>
> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
> ---
>
> My emacs was using indentation level of 8 spaces and this patch convinced
> it to use the correct value.
>
> I set c-basic-offset set to tab-width in my ~/.spacemacs which perhaps
> isn't the wisest thing to do, but since there's a .dir-locals.el we can
> make the editor always do the right thing.
>
>  .dir-locals.el | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/.dir-locals.el b/.dir-locals.el
> index 3ac0cfc6f0d6..8e47418c5996 100644
> --- a/.dir-locals.el
> +++ b/.dir-locals.el
> @@ -1,2 +1,3 @@
>  ((c-mode . ((c-file-style . "stroustrup")
> -	    (indent-tabs-mode . nil))))
> +	    (indent-tabs-mode . nil)
> +	    (c-basic-offset . 4))))

Ping?

-- 
Thiago Jung Bauermann
IBM Linux Technology Center


Re: [Qemu-devel] [PATCH] .dir-locals.el: Explicitly set indentation level
Posted by Markus Armbruster 6 years, 7 months ago
Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com> writes:

> At least in some configurations, setting c-file-style is not enough to
> conform to the QEMU coding style, so explicitly set c-basic-offset as well.
>
> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
> ---
>
> My emacs was using indentation level of 8 spaces and this patch convinced
> it to use the correct value.
>
> I set c-basic-offset set to tab-width in my ~/.spacemacs which perhaps
> isn't the wisest thing to do, but since there's a .dir-locals.el we can
> make the editor always do the right thing.

I think some of this rationale should be worked into the commit message
if we decide the patch makes sense.

>  .dir-locals.el | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/.dir-locals.el b/.dir-locals.el
> index 3ac0cfc6f0d6..8e47418c5996 100644
> --- a/.dir-locals.el
> +++ b/.dir-locals.el
> @@ -1,2 +1,3 @@
>  ((c-mode . ((c-file-style . "stroustrup")
> -	    (indent-tabs-mode . nil))))
> +	    (indent-tabs-mode . nil)
> +	    (c-basic-offset . 4))))

Style "stroustrup" already specified c-basic-offset 4.  If we need to
specify this setting a second time, why don't we need to specify the
style's other settings a second time, too?

Re: [Qemu-devel] [PATCH] .dir-locals.el: Explicitly set indentation level
Posted by Thiago Jung Bauermann 6 years, 7 months ago
Markus Armbruster <armbru@redhat.com> writes:

> Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com> writes:
>
>> At least in some configurations, setting c-file-style is not enough to
>> conform to the QEMU coding style, so explicitly set c-basic-offset as well.
>>
>> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
>> ---
>>
>> My emacs was using indentation level of 8 spaces and this patch convinced
>> it to use the correct value.
>>
>> I set c-basic-offset set to tab-width in my ~/.spacemacs which perhaps
>> isn't the wisest thing to do, but since there's a .dir-locals.el we can
>> make the editor always do the right thing.
>
> I think some of this rationale should be worked into the commit message
> if we decide the patch makes sense.

Ok, I will send a v2 with the rationale in the commit message if it's
determined that the patch itself is ok.

>>  .dir-locals.el | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/.dir-locals.el b/.dir-locals.el
>> index 3ac0cfc6f0d6..8e47418c5996 100644
>> --- a/.dir-locals.el
>> +++ b/.dir-locals.el
>> @@ -1,2 +1,3 @@
>>  ((c-mode . ((c-file-style . "stroustrup")
>> -	    (indent-tabs-mode . nil))))
>> +	    (indent-tabs-mode . nil)
>> +	    (c-basic-offset . 4))))
>
> Style "stroustrup" already specified c-basic-offset 4.  If we need to
> specify this setting a second time, why don't we need to specify the
> style's other settings a second time, too?

I added (defvaralias 'c-basic-offset 'tab-width) to my configuration
file following a suggestion from the "Indentation Basics" page of
EmacsWiki¹, which is the first hit if you search for "emacs indentation
width".

Because of that, I assume a lot of people will have that option in their
configuration file. If there are other settings that are common and also
interfere with the stroustrup style I think it makes sense to include
them in .dir-locals.el as well.

-- 
Thiago Jung Bauermann
IBM Linux Technology Center


¹ https://www.emacswiki.org/emacs/IndentationBasics