Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Documentation Code Sample Indentation #114

Closed
lukebarton opened this issue Oct 8, 2015 · 9 comments
Closed

Documentation Code Sample Indentation #114

lukebarton opened this issue Oct 8, 2015 · 9 comments
Assignees
Labels

Comments

@lukebarton
Copy link
Contributor

I appreciate we all have our coding styles but the way you've laid out your config JS in the documentation is horrible. I don't mean to offend you if you came up with this style but it's the most unreadable way of laying out code that I've ever seen. It creates a barrier to adoption.

@jkphl
Copy link
Collaborator

jkphl commented Oct 8, 2015

Hi @lukebarton,

before I try to dig into your issue, would you please be so kind and post a screenshot of one of the code samples you're having troubles with? Just to make sure we're talking about the same display.

Thanks & cheers, Joschi

@jkphl jkphl self-assigned this Oct 8, 2015
@jkphl jkphl added the question label Oct 8, 2015
@lukebarton
Copy link
Contributor Author

screen shot 2015-10-08 at 17 13 38

screen shot 2015-10-08 at 17 13 27

@jkphl
Copy link
Collaborator

jkphl commented Oct 8, 2015

Thanks for this proof that we're talking about the same thing.

I'm sorry to hear that you're having trouble with the code formatting. In fact, this is the first time the layout gets criticized. On the contrary, I had quite some people praising the formatting in the past, and my whole developer team is totally used to layout code like this. I guess — as so often — it all depends on one's habits. Having said that, please understand that I will wait for some more complaints like your's before I consider changing the layout. In the meantime, you could use the editor of your choice to auto-reformat the code samples to your liking.

Cheers, Joschi

@jkphl jkphl closed this as completed Oct 8, 2015
@lukebarton
Copy link
Contributor Author

Is your whole developer team used to this layout because you imposed it on them, rather than selecting a commonly used coding standard that already exists?

The coding habits of the maintainer of an open source repository have an very important role to play when selecting an open source project to depend on. Quite obviously, you've chosen to ignore common conventions in favour of your own in this case and it begs the question: "What other off-piste stuff is going to crop up making this project difficult to work with, modify, or fix, in the future?"

Anyway, perhaps you don't value my point in any case.

@jkphl
Copy link
Collaborator

jkphl commented Oct 9, 2015

I feel this discussion has a strong tendency towards getting pointless, so please excuse that I'm keeping it short.

My fellow developers and I evolved and silently agreed on our way of formatting our sources because we like(d) it this way and it fits our needs best. I don't think I have to justify that further.

When I wrote my first answer to you yesterday, my neighbour happened to give me a visit. He is a developer as well, but we never worked together so far. I showed him the screenshots you posted without telling him what it was and asked him if he had any problems understanding it. "Not at all, quite the contrary!", he said. Then I told him that it was my code. He said that he had never seen source code formatted like this before, but he confirmed once more that he had no problems reading and understanding it. That's the typical reaction I had so far.

You are the first person complaining about this. You say you don't mean to offend anyone, but in fact that's what you do. You don't provide any constructive suggestion, you don't even explain what exactly troubles you so much (I suppose it's the alignment of values, right?), but you insist on complaining. Sorry mate, but as long as that is your strategy you're definitely not the kind of user I'm putting my spare time into these projects for.

Never mind. Cheers, Joschi

@lukebarton
Copy link
Contributor Author

Suggestion: Leave arcane coding standards at home.

Follow something out there, a quick google:
http://javascript.crockford.com/code.html
https://github.com/felixge/node-style-guide

FWIW: Similarly, I shared one of the screenshots with a bunch of highly-regarded developers, who didn't find it as much a positive revelation as your neighbour.

To put my complaint into more concrete terms:

  1. The way you align all the values means that at short key depths, the values are far to far from the keys in comparison to the longest key.

Why is this a problem?
It's hard to understand a value's relation to a key because it's so far away from the other. You must scan back and forth left to right, with a huge chunk of whitespace in between, making it harder to track along a line.

Column aligning values manageable if you align ONE level of the structure, but you're aligning the entire structure's values at a shared level, regardless of depth (except for the closing brace, somehow you allow that to match the key, which isn't even in close proximity to it's opening brace).

Which segways nicely into:
2) Having the values independent of the object structure prevents you from easily understanding the context of the value within the data structure, or said another way: it's harder to understand the context of the value with relation to the data structure, because it's unrelated to the indentation level, unlike the key. Whilst scanning left and right across the whitespace, to understand the context of a value I have to manually parse the data's structure, looking at keys to understand the structure, trying to reconcile them with values (or opening braces, which contain all of the values underneath them, until I don't see anything then I have to scan all the way across to the left of the screen to hopefully find the closing brace of the type I'm expecting).

  1. If someone adds a key which increases to the total indentation level, they'll need to indent the entire set of values to line them up with your new value column, meaning git blame now has their name against all these lines, because they wanted to add something as simple as one value to the structure. This makes code review harder, since a reviewer should check all the lines edited, where this edit could have been avoided since it's whitespace only. It's easy to spot a mistake if the change is obvious. Whitespace changes can give the impression of 'oh it's just a whitespace change' and the reviewer will gloss over it, missing a mistake which may have been introduced.

In this code example (from your docs), there is a parse error. Line 4 is missing an opening brace?

// «css» sprite with Sass stylesheet resource

var config                  = {
    mode                    :
        css                 : {         // Create a «css» sprite
            render          : {
                scss        : true      // Render a Sass stylesheet
            }
        }
    }
}

Would that not be more easy to spot if the code was formatted in a more traditional style?

// «css» sprite with Sass stylesheet resource

var config = {
    mode : 
        css : {                // Create a «css» sprite
            render : {
                scss : true    // Render a Sass stylesheet
            }
        }
    }
}

We're talking about a very simple object here. If this mistake can be made (multiple times I might add) then consider a larger object with a more complex structure. It's not going to get any easier.

I submitted a pull request to update your docs.

@hamishtaplin
Copy link

I wasn't surprised that there was an issue raised about the formatting, it's really difficult to read if you aren't used to it.

+1 to adopting a more standard style of indentation.

@lone-star
Copy link

+1 for the point about git blame. It's not practical for reviewing history.

@techjanitor
Copy link

lmao

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants