> On 23 Nov 2017, at 14:55, Frediano Ziglio <fziglio@xxxxxxxxxx> wrote:
>
>>>
>>> On 23 Nov 2017, at 12:26, Frediano Ziglio <fziglio@xxxxxxxxxx> wrote:
>>>
>>>>
>>>> From: Christophe de Dinechin <dinechin@xxxxxxxxxx>
>>>>
>>>> Signed-off-by: Christophe de Dinechin <dinechin@xxxxxxxxxx>
>>>> ---
>>>> include/spice-streaming-agent/plugin.hpp | 8 ++++++++
>>>> 1 file changed, 8 insertions(+)
>>>>
>>>> diff --git a/include/spice-streaming-agent/plugin.hpp
>>>> b/include/spice-streaming-agent/plugin.hpp
>>>> index 1a58856..c370eab 100644
>>>> --- a/include/spice-streaming-agent/plugin.hpp
>>>> +++ b/include/spice-streaming-agent/plugin.hpp
>>>> @@ -149,6 +149,14 @@ extern "C" unsigned
>>>> spice_streaming_agent_plugin_interface_version;
>>>> */
>>>> extern "C" SpiceStreamingAgent::PluginInitFunc
>>>> spice_streaming_agent_plugin_init;
>>>>
>>>> +#define SPICE_STREAMING_AGENT_PLUGIN(agent) \
>>>> + __attribute__ ((visibility ("default"))) \
>>>> + unsigned spice_streaming_agent_plugin_interface_version = \
>>>> + SpiceStreamingAgent::PluginInterfaceVersion; \
>>>> + \
>>>> + __attribute__ ((visibility ("default"))) \
>>>> + bool spice_streaming_agent_plugin_init(SpiceStreamingAgent::Agent*
>>>> agent)
>>>> +
>>>> #endif
>>>>
>>>> #endif // SPICE_STREAMING_AGENT_PLUGIN_HPP
>>>
>>> Surely helps. Some notes:
>>> - spice_streaming_agent_plugin_interface_version is not const so I assume
>>> you want to allow to change it;
>>
>> No, it’s that if you make it const, the compiler may optimize it away.
>> There are two ways I can avoid that:
>> 1. Use its address
>> 2. Check that it won’t eliminate it if the __attribute__ above is used.
>>
>
> The attribute and the extern "C" should give the compiler quite lot of
> hints (more the second).
>
>>> - the attribute is GCC syntax but can be changed if needed;
>>
>> This was one reason to put it in a macro.
>>
>>
>>> - I know we use that style of indentation for line continuation but I
>>> honestly prefer to not have that indentation preferring a " \" at the
>>> end. This as current style:
>>> - is hard to maintain. Currently is already broken as last like is
>>> wrong;
>>
>> It’s actually the other way round with Emacs, which auto-indents
>> trailing \ (also has a C-c C-\ command (c-backslash-region).
>>
>> The last line is just too long, I will split it.
>>
>>> - it make copy&paste harder unless you indent always at a given
>>> standard column;
>>
>> Again, the problem goes the opposite way if you use Emacs.
>>
>>> - make email patch potentially hard to read.
>>
>> Why? Are you using a proportional font when reading patch emails?
>>
>
> If lines are long they'll all split in 2 lines.
> Also if code needs to be indented again there will be a lot of lines
> of difference just to change indentation forcing people to apply the
> patch and use some options to ignore spaces which will potentially can
> hide other unwanted space changes.
I don’t see how any of this is specific to trailing backslashes.
You could say the same thing for reindenting code because you
added an ‘if’.
>
>> In any case, that looks like a personal preference, and it goes the
>> opposite way for me for a variety of reasons:
>> - Your reasons backwards (when using Emacs to write and read patches)
>> - Emacs Is Always Right™
>> - Having \ at a known location helps me “put it out of the way” while
>> reading.
>>
>>
>> Cheers,
>> Christophe
>>
>
> Half of your reasoning are based on the editor used, if I remove
> Emacs from them basically you agree with me :-)
All of our respective reasonings are based on personal preferences.
Those, in turn, derive from the tools we use. I am not trying to make
them anything else than personal preferences. I was only explaining
that my editor choice happens to turn 100% of your arguments backwards.
But no, I don’t agree that code with misaligned trailing \ looks better
or is easier to read than aligned code. It makes it hard to read for me,
and I see no real reason to write code I find hard to read :-)
>
> Frediano
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel