ProcessWire icon indicating copy to clipboard operation
ProcessWire copied to clipboard

Core code indenting, whitespaces

Open somatonic opened this issue 11 years ago • 5 comments

Ryan, what do you think about cleaning up core code formatting.

Currently there's mixing of indent using tabs and spaces and whitespace at end of lines.

I found it hard to contribute to core code, as I use Sublime text in a way that cleans up stuff on save. So when making changes, pull requests include lots of changes that aren't really code changes.

somatonic avatar Jan 19 '14 13:01 somatonic

Interesting, there are more benefits then the valid point Soma is taken. Readability of code, inline elements don't take extra white-space space, easier to maintain.

Da-Fecto avatar Jan 19 '14 16:01 Da-Fecto

All of the code is supposed to be indented using tabs. In the last year I switched to using not just VIM, but a mixture of PHPStorm and VIM. I think that I didn't originally have PHPStorm configured for tabs across all file types, so ended up with some mixed indenting on occasion (especially in JS files). I fix any instance I find of it, to make sure everything uses tabs. But if you come across any files that aren't, or that are mixed, please let me know so that I can fix it.

As for whitespace at the end of a line, I've not paid attention to that one. When I run a diff, I always have it ignore whitespace anyway. In GItHub, you just add "?w=1" to see a diff with whitespace ignored. But would certainly prefer a consistency in whitespace at the end of a line too. I'll check to see if PHPStorm has a setting to take care of that for me automatically.

ryancramerdesign avatar Jan 20 '14 10:01 ryancramerdesign

mixed indents

/wire/core/WireUpload.php /wire/core/Selector.php L316

/wire/modules/Fieldtype/FieldtypeRepeater/FieldtypeRepeater.module /wire/modules/Fieldtype/FieldtypeComments/FieldtypeComments.module /wire/modules/Inputfield/InputfieldFile/InputfieldFile.js /wire/modules/Inputfield/InputfieldPageAutocomplete/InputfieldPageAutocomplete.module /wire/modules/Inputfield/InputfieldPageName/InputfieldPageName.js /wire/modules/Jquery/JqueryCore/JqueryCore.module /wire/modules/Jquery/JqueryFancybox/JqueryFancybox.module /wire/modules/Jquery/JqueryMagnific/JqueryMagnific.module /wire/modules/Jquery/JqueryTableSorter/JqueryTableSorter.module /wire/modules/Jquery/JqueryUI/JqueryUI.module /wire/modules/Jquery/JqueryWireTabs/JqueryWireTabs.module /wire/modules/LanguageSupport/FieldtypeTextareaLanguage.module /wire/modules/LanguageSupport/FieldtypeTextLanguage.module /wire/modules/LanguageSupport/LanguageSupport.module /wire/modules/LanguageSupport/LanguageSupportFields.module /wire/modules/Process/ProcessModule/ProcessModule.js /wire/modules/Process/ProcessPageAdd/ProcessPageAdd.js /wire/modules/Process/ProcessPageList/ProcessPageList.js /wire/modules/Process/ProcessPageSearch/ProcessPageSearch.js /wire/modules/Process/ProcessTemplate/ProcessTemplate.js /wire/index.config.php

I haven't looked all css but there's some

Also the whitespace on line ending is on all files on many lines so I would just list all files :)

somatonic avatar Jan 21 '14 20:01 somatonic

We can use something like this one http://editorconfig.org and include the config in the repo, so everyone who wants to contribute will need to use the config :+1:

peterfoeng avatar Aug 21 '14 02:08 peterfoeng

@somatonic A lot has happened to the core since this issue was written. Is this still an issue?

isellsoap avatar Oct 01 '16 07:10 isellsoap