Cleanup code / Coding style

[Wurstnase]

For a better readability, I would start to restyle the code.

Some files using Tabs. Some spaces. Some are mixed.

Where you have PEP8 in Python, I have no idea what we could use in C.

Any idea is welcome.

PS: Autostyle is not an option. Sometimes you need to break rules.

[rollingdice]

I think using space is better than using tab. Developers use different tab rendering setting on their IDE. Using space would unify coding experience for all developer.

But whatever the final decision is, don't forget to notify devs to enable whitespace display on their IDE so they would notice our conventions on indenting.

Just my 2¢.

[triffid]

I think tab is better than space for exactly the same reason. Then I can set the tab width to whatever I'm comfortable with (4) and other developers can stick with their preference.

I believe there's a smoothie coding style guide on the wiki somewhere On 31 Mar 2016 6:28 pm, "Banitama Supartha" notifications@github.com wrote:

I think using space is better than using tab. Developers use different tab rendering setting on their IDE. Using space would unify coding experience for all developer.

But whatever the final decision is, don't forget to notify devs to enable whitespace display on their IDE so they would notice our conventions on indenting.

Just my 2¢.

— You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub https://github.com/Traumflug/Teacup_Firmware/issues/204#issuecomment-203870973

[triffid]

Damn, wrong list, should be a teacup style guide somewhere :/ On 31 Mar 2016 6:31 pm, "Triffid Hunter" triffid.hunter@gmail.com wrote:

I think tab is better than space for exactly the same reason. Then I can set the tab width to whatever I'm comfortable with (4) and other developers can stick with their preference.

I believe there's a smoothie coding style guide on the wiki somewhere On 31 Mar 2016 6:28 pm, "Banitama Supartha" notifications@github.com wrote:

I think using space is better than using tab. Developers use different tab rendering setting on their IDE. Using space would unify coding experience for all developer.

But whatever the final decision is, don't forget to notify devs to enable whitespace display on their IDE so they would notice our conventions on indenting.

Just my 2¢.

— You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub https://github.com/Traumflug/Teacup_Firmware/issues/204#issuecomment-203870973

[rollingdice]

Well that works, too. Using tab also reduce file size by a small amount.

[triffid]

Pretty sure the filesize of whitespace in code is pretty irrelevant in this modern age :p On 31 Mar 2016 6:34 pm, "Banitama Supartha" notifications@github.com wrote:

Well that works, too. Using tab also reduce file size by a small amount.

— You are receiving this because you commented. Reply to this email directly or view it on GitHub https://github.com/Traumflug/Teacup_Firmware/issues/204#issuecomment-203873095

[Traumflug]

Some files using Tabs. Some spaces. Some are mixed.

The goal is to have spaces only and indentation by 2 of them. Also to have no lines longer than 80 characters.

Replacing all tabs with spaces is a snap, most code editors have a dedicated function for it. It would have been done long ago already, if it wasn't Git dealing so poorly with pure whitespace changes. When rebasing a branch, pure whitespace changes are considered to be a change, so one gets conflicts all over the place and conflicts often as large as the whole file. That's why I held back with a general move so far.

The current strategy is to bring in all new code with spaces, only, keeping existing code as-is. This minimises conflicts, they happen only where they'd happen anyways.

One strategy to get the move done without these hard to resolve conflicts might be to change only every fifth line of code per commit, splitting the move into 5 commits. Then Git should split conflicts into small chunks, making them easily resolvable. Didn't investigate this deeper so far.

[Wurstnase]

Maybe we could something like this? http://editorconfig.org/

[Traumflug]

Maybe we could something like this? http://editorconfig.org/

As I tried to explain before, keeping the style isn't the problem.

[Wurstnase]

Here a simple way for conversion. (python3)

from pathlib import Path

start_with_line = 0
convert_any_x_line = 5

file_dir = 'path_to_teacup'
file_types = ['*.c', '*.h']

file_list = []

dir_obj = Path(file_dir)

for file_type in file_types:
    list_file = list(dir_obj.glob(file_type))
    file_list.extend(list_file)

for file in file_list:
    tab_counter = 0
    with file.open('r') as f:
        try:
            file_str = f.readlines()
        except Exception:
            pass
    print(file)
    for i, line in enumerate(file_str):
        if not (i + start_with_line) % convert_any_x_line:
            find_tab = file_str[i].find('\t')
            if find_tab >= 0:
                file_str[i] = file_str[i].replace('\t', '  ')
                tab_counter += 1
                print('{}:{}'.format(i, tab_counter))
                print(file_str[i])

    if tab_counter:
        with file.open('w') as f:
            try:
                f.writelines(file_str)
            except Exception:
                print(f)

[Wurstnase]

Tested a rebase on arm-stm32f411-port with https://github.com/Traumflug/Teacup_Firmware/tree/issue_204 Looks promising. No errors at all.

[Traumflug]

Now please try to rebase another branch on top of arm-stm32f411-port. Like

git checkout arc-support
git rebase arm-stm32f411-port

To my experience, each single line of changed code causes a large conflict, then.

[Wurstnase]

git checkout arc-support
git rebase --irgnore-whitespace arm-stm32f411-port

works.

Edit: Ok, I think I see the issue... This is ugly...

[phord]

Some helpful tips can be found here.

https://gist.github.com/eevee/6721177

Also you can use a filter-branch script to "expand" all source code consistently whenever you need to merge, rebase or cherry-pick. It is not ideal, and it interferes with history, but there it is.

[Wurstnase]

I found the same filter some days ago and start with some test. I will report when I get some experience with that.

Phil Hord notifications@github.com schrieb am Mo., 4. Apr. 2016 um 15:00 Uhr:

Some helpful tips can be found here.

https://gist.github.com/eevee/6721177

Also you can use a filter-branch script to "expand" all source code consistently whenever you need to merge, rebase or cherry-pick. It is not ideal, and it interferes with history, but there it is.

— You are receiving this because you authored the thread.

Reply to this email directly or view it on GitHub https://github.com/Traumflug/Teacup_Firmware/issues/204#issuecomment-205286742

[Wurstnase]

Hmmm... can't get the filter to work. I made a .gitconfig. Put it into the mainfolder. Expand files manually. Push and forced pull. Rebase with -Xrenomalize -Xtheirs works, but without expand the new files.

Looks like the .gitconfig-filter doesn't applied for new files, or (my guess) I'm doing something wrong.

[phord]

Did you add attributes for .c and .h files in .gitattributes?

*.c  filter=spabs
*.h  filter=spabs

[Wurstnase]

Yes, I have.

[dcousens]

To see code diffs without a gigantic whitespace diff, ?w=0 on the end of the URL (yay GitHub).

See this pull request for an example.

And with the white space: https://github.com/Traumflug/Teacup_Firmware/pull/226/files