The blame

[phidebian]

Hi All, when submitting PR (like parse.c yesterday) I got blammed, I mean git blammed :) apparently.

Long story short, I work in an environment that ban tabs (syntax directed editor, construct template for if() for() {} etc..) precisely to avoid the tab saga discussion, no tabs mean spaces only, and 1 missing space (or too much space) is visible, and a diff'er can highlight a space, it can not highlight a tab. So a zealous editor (not me on purpose) can goof around with tabs.

Whence a bug is fixed I got to do an whole process to put my work back to the ksh93 coding style, this is manually done and generally involve a dumb pristine vi so I can tabify my cut/paste etc, a cumbersome task, and surely not bullet proof, the yesterday PR was a 1liner a=1; kind yet my editor rendered then next line with a tab/space faulty apparently.

My question is, what source canonicalisation tools are you using, (in my other projects I use astyle, gdb team use llvm) whatever, any source got to be canonicalised before getting pushed.

I got blamed for a wrong space in a tab in sparse.c, I don't really get it because sparse.c itself has 202 fault space/tab illegal combo meaning so far I can't find a way to automate your coding style.

This reveal the buggy tab/space combo.

• 🡆 means <space><tab>
• 🡄 means <tab><space>
• More generally the arrow is the faulty <space>

$ cd /d1/ksh/src/cmd/ksh93/sh
$ sed -n '
s/[ ][  ]/\xf0\x9f\xa1\x84      /gp
s/[     ][ ]/\xf0\x9f\xa1\x86   /gp
' parse.c 

This also catch the crap inside comment one could argue we don't care, but I bet git will still blame me if I goof around in comment (not sure about that).

Yet lines like

	🡄      🡄      case '&':

Are surely as bad as what I submitted, but may be if they where entered by dgk\bs\bs\bsgod then they are legal ?

parse.c itself has 202 faulty combo

$ sed -n '
s/[ ][  ]/\xf0\x9f\xa1\x84      /gp
s/[     ][ ]/\xf0\x9f\xa1\x86   /gp
' parse.c  | wc -l
202

and in sh/ only I got this

                        defs.c 30
                       tdump.c 34
                      expand.c 34
                      timers.c 34
                     deparse.c 34
                       pmain.c 36
                    trestore.c 36
                   waitevent.c 36
                        fcin.c 44
                      shcomp.c 52
                       fault.c 58
                      nvtree.c 60
                       arith.c 66
                    subshell.c 66
                        main.c 72
                      nvtype.c 76
                      string.c 88
                        args.c 96
                      nvdisc.c 118
                       array.c 132
                        init.c 138
                         lex.c 164
                        jobs.c 178
                       parse.c 202
                        name.c 206
                     streval.c 208
                        path.c 224
                          io.c 230
                       macro.c 236
                         xec.c 342

I also noticed that (don't remember in which patch though) one did align declarations i.e

	int i;
	long int j;

becomes

	int		i;
	long int	j;

I personnally like it a lot, but then it makes a diff with ancient code, and in some way not different with adding/removing <space><tab> goofs ?

Is that illegal to sanitize some code as we bump into when fixing code?
I guess this is a never ending discussion, but for tabs I guess we should allow diffs, git is able to ignore tab/space (at least for diff) OR we should have a sanitize process so I could just pipe my source into to produce what you want.

OR may be I just miss a technic that you use to avoid this kind of problems so let me know then will do it.

[hyenias]

Lots of questions. Lots of suggestions. Have you read the https://github.com/ksh93/ksh/blob/dev/README.md which is the main page for this repo? See Policy section especially Policy 7.

There is a lot of drama behind some of those policies decisions and lessons learned from the failed ksh2020 project (at least from my understanding as I can only speak for myself). The ksh2020 project tried implementing/imposing many of those standardization/uniformity concepts, guidelines, and tools to the detriment of the community and ksh's code base. Yes, I do realize that a tool is a tool and nothing more; it is the hand that wields it.

I am grateful that @McDutchie has taken over leadership and maintenance of the KornShell interpreter. Additionally, @McDutchie has stated some more about coding styles at #614 which you are aware.

To help address this "The blame" discussion; no, I personally do not use any tool nor am I aware of others using any on a routine fashion to alter/check code in an automated way. Yes, every line of code is hand coded--line-by-line using a human's ability to try to mimic the author(s) surrounding code style until such time @McDutchie alters this repo's desires.

I think of it this way: Michelangelo would not use an automated tool to produce one of his masterpiece sculptures. Only great care would be used to shape such works of art.

[McDutchie]

I like using git blame as a documentation tool, but I'm not so much into blaming people. I'm grateful for your contributions and I'd much rather have them with some formatting issues to fix than not have them at all! Of course I'd appreciate it if you could make my life easier, but if that is too burdensome please don't let it stop you. I will simply keep fixing the formatting and style myself.

My question is, what source canonicalisation tools are you using

None. This is all written by hand, the old fashioned way. I fix everything by hand as well.

Having said that, I just tried astyle, and it looks like astyle --indent=tab matches the pre-existing style very closely. So if you use that on your changes, you should be fine. I just tried running

astyle --indent=tab <src/cmd/ksh93/sh/xec.c >xec.c.new

and Iike the results.

I know there are lots of pre-existing formatting inconsistencies. But fixing them all would make it harder to find why a change was introduced, as git blame would show the commit that changed the formatting instead of the one that introduced our changes. We rely pretty heavily on this; the main documentation for the reasoning behind the changes is the commit messages.

However, I'd like to avoid introducing new formatting inconsistencies where possible, and where we change existing code anyway we might as well fix the formatting too.

I like tab indentation; I think it's logical and practical, and it works for me.

Here's a rough idea for a process, maybe it might work for you:

1. Just make your changes and don't worry about formatting, but keep a backup of the original.
2. Run your changed files through astyle --indent=tab, creating a new copy.
3. Use your editor's block copying feature to copy over your reformatted changes into the backup of the original, leaving the rest intact.
4. Replace your changed file with the modified backup. Now you have fixed your formatting.
5. Make a PR :)

[phidebian]

Thanx you all, well I am ignorant of ksh2020 just saw reference to it, I think I saw it once in my distro couple years ago and disapear, but at that time I was not using ksh93 much (was using my own version of ksh88 :-) )

I discover git blame, actually @McDutchie mentionned it a couple of time, and it took some time to makes the neuronal conexion to my brain (learning curve), so I think I will run that before submitting anything.

I love astyle (but is far less powerfull than the LLVM tools), but I rekon it is not ideal, because it would fix all the <space>/<tab> current glitch to the detriment of acheological git diff (I like that archeological). When used from project start, (or project fork with no intention to return to previous points) then choosing a canonical style mean that 'therorically' you can display (build) in your own style then you can put it back to what it was lossless

I will try as well for my added code a simple region select pipe to astyle (I will need to tweak it more to get the { style) but I may be close to find a way to normalise the push process.

Again thanx for all your explanations.

[phidebian]

I think I have refined the idea, when I have a file updated (ongoing work) not even git added and then not even committed, I can do this

$ git diff --color=always src/cmd/ksh93/bltins/print.c  | 
sed 's/^ /./;s/ /\xf0\x9f\xa2\xac/g'  | more
diff🢬--git🢬a/src/cmd/ksh93/bltins/print.c🢬b/src/cmd/ksh93/bltins/print.c
index🢬b6e321f7..b26d3019🢬100644
---🢬a/src/cmd/ksh93/bltins/print.c
+++🢬b/src/cmd/ksh93/bltins/print.c
@@🢬-38,14🢬+38,15🢬@@
.
.union🢬types_t
.{
-       unsigned🢬char   c;
+🢬🢬🢬    unsigned🢬char   c;  <== Added line with <illegal spaces>
.       short           h;
-       int             i;
+       int     🢬🢬      i;  <== added line with <illegal spaces>
.       long            l;
.       Sflong_t        ll;
.       Sfdouble_t      ld;
-       double          d;
+🢬🢬🢬🢬🢬🢬🢬🢬double         d;
.       float           f;
+       float           properly_added_line; <== Added line correctly added
.       char            *s;

I can focus on the + line the one I added and can spot right away crappy <spaces>

I will hide all that in my git-diff wrapper.

Hope I will do better on next PR :)