Closed paboyle closed 7 years ago
Got it. I will find a solution for when the same files appear messy on my editor. Not because of the original formatting style but because of the treatment of spaces. Reformatting of some files was not a matter of code style taste but readability on my side too. I will not apply any LLVM format anymore.
Hi,
In general, I don't know if there is a way to enforce this in a foolproof manner except for the self-dicipline.
FWIW, 1) specify preferred indentation by hooks and/or script(s) 2) using whitespace-aware tools (diff3,meld,...) for merge
Could help if/when this happens. I recently went through merging Gparity branch in CPS, and it'd been nearly impossible without 2).
Chulwoo
On Sat, 22 Oct 2016, Guido Cossu wrote:
Got it. I will find a solution for when the same files appear messy on my editor. Not because of the original formatting style but because of the treatment of spaces. Reformatting of some files was not a matter of code style taste but readability on my side too. I will not apply any LLVM format anymore.
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.[AErp_-CS25akvdM8NRUyI9PubcFPRXtaks5q2bmsgaJpZM4Kds34.gif]
Chulwoo Jung Physics Department Brookhaven National Laboratory U.S.A. chulwoo@bnl.gov 1-631-344-5254
Ok, I have noticed something. Peter, it sounds like your editor is indenting using a mix of spaces and tabs (are you using two different editors?). Depending on how the editor interprets tabs some files looked pretty messed up (e.g. https://www.dropbox.com/s/lc50pb5cbhromau/Screenshot%202016-10-26%2018.33.15.png?dl=0). It can be a real headache to follow the scope hierarchy.
I have determined that the code has been originally indented with tabs having a width of 8. So setting my editor to space indents of 2 and a tab width to 8 the code looks alright.
I think it would be beneficial before a next release to convert all the tabs to 8 spaces, like that the code would look the same wherever it is opened. In general for the future I would say that it's better to only do space-indentation (which is what most code editors are doing for C/C++).
I am happy to audit and prepare for conversion but it would be good to have feedback to be sure everybody would be fine with this kind of solution.
I'm not happy with auto conversion. I agree autopreparing a list of files with naked \t in them and manually cleaning makes sense. I'm happy to do this since I'm not happy with the format rot we've been acquiring and a global edit is required.
A small comment -- after some thought on indentation and tools, I'm prepared to drop our "namespace" declarations into macros that look like legal C++ statements, including a semicolon to avoid fooling C++ aware editors
GRID_BEGIN_NAME_SPACE(); GRID_END_NAME_SPACE();
to prevent attempted indentation of intervening code. This also leaves code indented by the same amount wether or not we drop the "QCD" subspace.
I totally second that, that’s exactly what I am doing in LatAnalyze and the measurement code (cf. https://github.com/paboyle/Grid/blob/feature/hadrons/programs/Hadrons/Global.hpp https://github.com/paboyle/Grid/blob/feature/hadrons/programs/Hadrons/Global.hpp). In principle you don’t need the empty argument (). However, it is a global edit we should be all synchronised when this happens or a lot of conflicts will happen.
Best,
Antonin
On 31 Oct 2016, at 15:15, Peter Boyle <notifications@github.com mailto:notifications@github.com> wrote:
A small comment -- after some thought on indentation and tools, I'm prepared to drop our "namespace" declarations into macros that look like legal C++ statements, including a semicolon to avoid fooling C++ aware editors
GRID_BEGIN_NAME_SPACE(); GRID_END_NAME_SPACE();
to prevent attempted indentation of intervening code. This also leaves code indented by the same amount wether or not we drop the "QCD" subspace.
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/paboyle/Grid/issues/60#issuecomment-257321489, or mute the thread https://github.com/notifications/unsubscribe-auth/AAJWuDoR71y_-v_2D2AGV-9-cAuKO364ks5q5gX9gaJpZM4Kds34.
https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-11e6-95fc-7290892c7bb5.png https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png https://github.com/paboyle/Grid https://github.com/paboyle/Grid/issues/60#issuecomment-257321489
I have no particular preference for this choice, so I can go on with it.
I plan to do this namespace change at some point soon. I'm clearing the decks to get a bunch of old small items like this done.
I want to be clear on this.
My source code is written to be readable by me.
I do NOT appreciate to reformatting source files, and committing them, especially if this is done brainlessly by an automatic tool, but in any case the style of the author is key. It does not matter if you do not like my style and choices.
A complete mess has been made of several source key files, and further dozens of files have been pointlessly changed in a way that violates my personal preference of not acquiring high levels of indentation when entering the Grid or QCD namespace.
This floating indent, combined with a tool based application of 80 character wrap creates unreadable code from code that was previously easily readable by the author.
Further, readability is in the judge of the prime author of a given area of the code, and it is rude and inappropriate to reformat without consultation and agreement.
I am now spending several hours reverting code with this waste of time created by thoughtless action.
The worst case is the thoughtless application of automatic formatting to a critical file with braces and scopes in ifdef's that hopeless confused the formatting.
You should NEVER be committing without first applying git diff and satisfying yourself that your are in complete control of these changes with a very few lines deliberately modified with genuine purpose.