Re: [Mesa-dev] [PATCH mesa] util: remove unnecessary random whitespaces
Ian Romanick writes: > On 10/25/2018 05:13 AM, Eric Engestrom wrote: >> On Thursday, 2018-10-25 17:54:16 +1100, Timothy Arceri wrote: >>> On 25/10/18 7:42 am, Ian Romanick wrote: On 10/23/2018 04:15 AM, Eric Engestrom wrote: > Suggested-by: Timothy Arceri >>> >>> Um no :P I suggested you fix the formatting in your patch to match the Mesa >>> style. >> >> Right, sorry, you suggested fixing the formatting, but not the fix >> I went with, so I should've dropped this tag. >> >>> > Signed-off-by: Eric Engestrom > --- > Timothy, I opted to remove them all instead of adding even more, as it > would break again next time something changes (the set_foreach() one was > already broken before my patch for instance) and result in lots of > unnecessary churn for seemingly no gain, and I don't like hiding the > backslash away (it hinders readability). NAK... we use this formatting everywhere in Mesa. The point is to move the \ characters out of the way. When you're trying to read a multi-line macro, they are distracting, so it is nice to move them over. >> >> I don't have the same opinion, but respecting mesa style is the point >> here, so I added those whitespace chars, squashed them in the previous >> patches, and pushed them. >> >> Sorry for this patch :) > > It's always worth a try. :) Everyone has aspects of the Mesa style that > they don't like. Sending out a patch like this is a good way to test > the waters about changing the style. It does happen from time to time. > It's better to do it like this than to try to sneak it in a big patch > series. I would love to see this style change. emacs in my experience does horrible things to these weird (inconsistently-)aligned \s, so I have to manually align them and fail. signature.asc Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH mesa] util: remove unnecessary random whitespaces
On 10/25/2018 05:13 AM, Eric Engestrom wrote: > On Thursday, 2018-10-25 17:54:16 +1100, Timothy Arceri wrote: >> On 25/10/18 7:42 am, Ian Romanick wrote: >>> On 10/23/2018 04:15 AM, Eric Engestrom wrote: Suggested-by: Timothy Arceri >> >> Um no :P I suggested you fix the formatting in your patch to match the Mesa >> style. > > Right, sorry, you suggested fixing the formatting, but not the fix > I went with, so I should've dropped this tag. > >> Signed-off-by: Eric Engestrom --- Timothy, I opted to remove them all instead of adding even more, as it would break again next time something changes (the set_foreach() one was already broken before my patch for instance) and result in lots of unnecessary churn for seemingly no gain, and I don't like hiding the backslash away (it hinders readability). >>> >>> NAK... we use this formatting everywhere in Mesa. The point is to move >>> the \ characters out of the way. When you're trying to read a >>> multi-line macro, they are distracting, so it is nice to move them over. > > I don't have the same opinion, but respecting mesa style is the point > here, so I added those whitespace chars, squashed them in the previous > patches, and pushed them. > > Sorry for this patch :) It's always worth a try. :) Everyone has aspects of the Mesa style that they don't like. Sending out a patch like this is a good way to test the waters about changing the style. It does happen from time to time. It's better to do it like this than to try to sneak it in a big patch series. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH mesa] util: remove unnecessary random whitespaces
On Thursday, 2018-10-25 17:54:16 +1100, Timothy Arceri wrote: > On 25/10/18 7:42 am, Ian Romanick wrote: > > On 10/23/2018 04:15 AM, Eric Engestrom wrote: > > > Suggested-by: Timothy Arceri > > Um no :P I suggested you fix the formatting in your patch to match the Mesa > style. Right, sorry, you suggested fixing the formatting, but not the fix I went with, so I should've dropped this tag. > > > > Signed-off-by: Eric Engestrom > > > --- > > > Timothy, I opted to remove them all instead of adding even more, as it > > > would break again next time something changes (the set_foreach() one was > > > already broken before my patch for instance) and result in lots of > > > unnecessary churn for seemingly no gain, and I don't like hiding the > > > backslash away (it hinders readability). > > > > NAK... we use this formatting everywhere in Mesa. The point is to move > > the \ characters out of the way. When you're trying to read a > > multi-line macro, they are distracting, so it is nice to move them over. I don't have the same opinion, but respecting mesa style is the point here, so I added those whitespace chars, squashed them in the previous patches, and pushed them. Sorry for this patch :) > > > > > --- > > > src/util/hash_table.h | 6 +++--- > > > src/util/set.h| 6 +++--- > > > 2 files changed, 6 insertions(+), 6 deletions(-) > > > > > > diff --git a/src/util/hash_table.h b/src/util/hash_table.h > > > index b96cd6146960a6a6f8a1..b9c9dfa01aeaa5e9cac1 100644 > > > --- a/src/util/hash_table.h > > > +++ b/src/util/hash_table.h > > > @@ -139,9 +139,9 @@ _mesa_fnv32_1a_accumulate_block(uint32_t hash, const > > > void *data, size_t size) > > >* an entry's data with the deleted marker), but not against insertion > > >* (which may rehash the table, making entry a dangling pointer). > > >*/ > > > -#define hash_table_foreach(ht, entry) \ > > > - for (struct hash_entry *entry = _mesa_hash_table_next_entry(ht, > > > NULL); \ > > > -entry != NULL; \ > > > +#define hash_table_foreach(ht, entry) \ > > > + for (struct hash_entry *entry = _mesa_hash_table_next_entry(ht, > > > NULL); \ > > > +entry != NULL; \ > > > entry = _mesa_hash_table_next_entry(ht, entry)) > > > static inline void > > > diff --git a/src/util/set.h b/src/util/set.h > > > index 3c9abfe77128292557ec..4307f4732fd4fde132a0 100644 > > > --- a/src/util/set.h > > > +++ b/src/util/set.h > > > @@ -96,9 +96,9 @@ _mesa_set_random_entry(struct set *set, > > >* insertion (which may rehash the set, making entry a dangling > > >* pointer). > > >*/ > > > -#define set_foreach(set, entry) \ > > > - for (struct set_entry *entry = _mesa_set_next_entry(set, NULL); \ > > > -entry != NULL; \ > > > +#define set_foreach(set, entry) \ > > > + for (struct set_entry *entry = _mesa_set_next_entry(set, NULL); \ > > > +entry != NULL; \ > > > entry = _mesa_set_next_entry(set, entry)) > > > #ifdef __cplusplus > > > > > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH mesa] util: remove unnecessary random whitespaces
On 25/10/18 7:42 am, Ian Romanick wrote: On 10/23/2018 04:15 AM, Eric Engestrom wrote: Suggested-by: Timothy Arceri Um no :P I suggested you fix the formatting in your patch to match the Mesa style. Signed-off-by: Eric Engestrom --- Timothy, I opted to remove them all instead of adding even more, as it would break again next time something changes (the set_foreach() one was already broken before my patch for instance) and result in lots of unnecessary churn for seemingly no gain, and I don't like hiding the backslash away (it hinders readability). NAK... we use this formatting everywhere in Mesa. The point is to move the \ characters out of the way. When you're trying to read a multi-line macro, they are distracting, so it is nice to move them over. --- src/util/hash_table.h | 6 +++--- src/util/set.h| 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/util/hash_table.h b/src/util/hash_table.h index b96cd6146960a6a6f8a1..b9c9dfa01aeaa5e9cac1 100644 --- a/src/util/hash_table.h +++ b/src/util/hash_table.h @@ -139,9 +139,9 @@ _mesa_fnv32_1a_accumulate_block(uint32_t hash, const void *data, size_t size) * an entry's data with the deleted marker), but not against insertion * (which may rehash the table, making entry a dangling pointer). */ -#define hash_table_foreach(ht, entry) \ - for (struct hash_entry *entry = _mesa_hash_table_next_entry(ht, NULL); \ -entry != NULL; \ +#define hash_table_foreach(ht, entry) \ + for (struct hash_entry *entry = _mesa_hash_table_next_entry(ht, NULL); \ +entry != NULL; \ entry = _mesa_hash_table_next_entry(ht, entry)) static inline void diff --git a/src/util/set.h b/src/util/set.h index 3c9abfe77128292557ec..4307f4732fd4fde132a0 100644 --- a/src/util/set.h +++ b/src/util/set.h @@ -96,9 +96,9 @@ _mesa_set_random_entry(struct set *set, * insertion (which may rehash the set, making entry a dangling * pointer). */ -#define set_foreach(set, entry) \ - for (struct set_entry *entry = _mesa_set_next_entry(set, NULL); \ -entry != NULL; \ +#define set_foreach(set, entry) \ + for (struct set_entry *entry = _mesa_set_next_entry(set, NULL); \ +entry != NULL; \ entry = _mesa_set_next_entry(set, entry)) #ifdef __cplusplus ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH mesa] util: remove unnecessary random whitespaces
On 10/23/2018 04:15 AM, Eric Engestrom wrote: > Suggested-by: Timothy Arceri > Signed-off-by: Eric Engestrom > --- > Timothy, I opted to remove them all instead of adding even more, as it > would break again next time something changes (the set_foreach() one was > already broken before my patch for instance) and result in lots of > unnecessary churn for seemingly no gain, and I don't like hiding the > backslash away (it hinders readability). NAK... we use this formatting everywhere in Mesa. The point is to move the \ characters out of the way. When you're trying to read a multi-line macro, they are distracting, so it is nice to move them over. > --- > src/util/hash_table.h | 6 +++--- > src/util/set.h| 6 +++--- > 2 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/src/util/hash_table.h b/src/util/hash_table.h > index b96cd6146960a6a6f8a1..b9c9dfa01aeaa5e9cac1 100644 > --- a/src/util/hash_table.h > +++ b/src/util/hash_table.h > @@ -139,9 +139,9 @@ _mesa_fnv32_1a_accumulate_block(uint32_t hash, const void > *data, size_t size) > * an entry's data with the deleted marker), but not against insertion > * (which may rehash the table, making entry a dangling pointer). > */ > -#define hash_table_foreach(ht, entry) \ > - for (struct hash_entry *entry = _mesa_hash_table_next_entry(ht, NULL); \ > -entry != NULL; \ > +#define hash_table_foreach(ht, entry) \ > + for (struct hash_entry *entry = _mesa_hash_table_next_entry(ht, NULL); \ > +entry != NULL; \ > entry = _mesa_hash_table_next_entry(ht, entry)) > > static inline void > diff --git a/src/util/set.h b/src/util/set.h > index 3c9abfe77128292557ec..4307f4732fd4fde132a0 100644 > --- a/src/util/set.h > +++ b/src/util/set.h > @@ -96,9 +96,9 @@ _mesa_set_random_entry(struct set *set, > * insertion (which may rehash the set, making entry a dangling > * pointer). > */ > -#define set_foreach(set, entry) \ > - for (struct set_entry *entry = _mesa_set_next_entry(set, NULL); \ > -entry != NULL; \ > +#define set_foreach(set, entry) \ > + for (struct set_entry *entry = _mesa_set_next_entry(set, NULL); \ > +entry != NULL; \ > entry = _mesa_set_next_entry(set, entry)) > > #ifdef __cplusplus > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH mesa] util: remove unnecessary random whitespaces
Suggested-by: Timothy Arceri Signed-off-by: Eric Engestrom --- Timothy, I opted to remove them all instead of adding even more, as it would break again next time something changes (the set_foreach() one was already broken before my patch for instance) and result in lots of unnecessary churn for seemingly no gain, and I don't like hiding the backslash away (it hinders readability). --- src/util/hash_table.h | 6 +++--- src/util/set.h| 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/util/hash_table.h b/src/util/hash_table.h index b96cd6146960a6a6f8a1..b9c9dfa01aeaa5e9cac1 100644 --- a/src/util/hash_table.h +++ b/src/util/hash_table.h @@ -139,9 +139,9 @@ _mesa_fnv32_1a_accumulate_block(uint32_t hash, const void *data, size_t size) * an entry's data with the deleted marker), but not against insertion * (which may rehash the table, making entry a dangling pointer). */ -#define hash_table_foreach(ht, entry) \ - for (struct hash_entry *entry = _mesa_hash_table_next_entry(ht, NULL); \ -entry != NULL; \ +#define hash_table_foreach(ht, entry) \ + for (struct hash_entry *entry = _mesa_hash_table_next_entry(ht, NULL); \ +entry != NULL; \ entry = _mesa_hash_table_next_entry(ht, entry)) static inline void diff --git a/src/util/set.h b/src/util/set.h index 3c9abfe77128292557ec..4307f4732fd4fde132a0 100644 --- a/src/util/set.h +++ b/src/util/set.h @@ -96,9 +96,9 @@ _mesa_set_random_entry(struct set *set, * insertion (which may rehash the set, making entry a dangling * pointer). */ -#define set_foreach(set, entry) \ - for (struct set_entry *entry = _mesa_set_next_entry(set, NULL); \ -entry != NULL; \ +#define set_foreach(set, entry) \ + for (struct set_entry *entry = _mesa_set_next_entry(set, NULL); \ +entry != NULL; \ entry = _mesa_set_next_entry(set, entry)) #ifdef __cplusplus -- Cheers, Eric ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
