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

Codestyle and formatting rules #34

Merged
merged 1 commit into from
Oct 31, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions .prettierrc
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
{
"tabWidth": 4,
"useTabs": false,
"trailingComma": "es5",
"semi": true,
"singleQuote": false,
"quoteProps": "consistent",
"trailingComma": "es5",
"bracketSameLine": true,
"bracketSpacing": true,
"arrowParens": "always",
"singleAttributePerLine": false,
"printWidth": 120,
"quoteProps": "consistent",
"overrides": [
{
"files": ["**/*.html","**/*.css"],
Expand All @@ -25,4 +28,4 @@
"options": { "parser": "json" }
}
]
}
}
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ Style elements for the scaffolding and the dynamic-tree nodes.

## New Views

The tree viewer can be extended with additional views. See [documentation](contributing.md).
The tree viewer can be extended with additional views. See [documentation](docs/contributing.md).

## Example

Expand Down
58 changes: 58 additions & 0 deletions docs/codestyle.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
# WikiTree code style

## Code style

we aim for consistency, generality, readability and reduction of git diffs. Similar language constructs are formatted with similar rules. Style configuration options are deliberately limited and rarely added. Previous formatting is taken into account as little as possible.
MichalVasut marked this conversation as resolved.
Show resolved Hide resolved

## Basic rules

| Rule | Value | Note |
| --- | :---: | --- |
| Line length | 120 characters | |
MichalVasut marked this conversation as resolved.
Show resolved Hide resolved
| Indentation | 4 spaces | |
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is my personal preference - I'm python coder and this is golden standard in python. I've thought about 2 spaces (also suggested by Google JS guidelines) and it's more compact, but I'm used to 4 spaces and those 2 doesn't look so clear and it's IMHO harder to orient, but thats what I feel.

I'm against using tabs

  • editors can be set to make spaces on tab key press
  • Github makes ver large spaces for tab
  • it doesn't really matter it you save some characters (1 character for tab vs N characters for every space)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. Two spaces is not enough, and we should definitely use four. I also agree they should be actual spaces. Editors can just do the replacement so the tab key itself works as expected. But storing tab characters in the file means everything gets out of alignment when viewed later.

The .prettierrc file that's currently in the repo sets tabWidth to two (2) spaces for HTML and CSS files. Is there a reason not to just have those files also use four-space indentation? (It also has an "override" for .js files, but that sets the tabWidth to the default 4; that's probably unnecessary.)

| Semicolons at the end of the line| yes | |
| Quote type | Double quotes | e.g. `myObj["propA"]` |
| Trailing comma | optional | *If you want to add a new property, you can add a new line without modifying the previously last line if that line already uses a trailing comma. This makes version-control diffs cleaner and editing code might be less troublesome.* [[source](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Trailing_commas)] |
| Bracket spacing | yes | e.g. `function xyz(arg1){ console.log(arg1) }` |
| Bracket line | yes | Put the > of a multi-line HTML (HTML, JSX, Vue, Angular) element at the end of the last line instead of being alone on the next line (does not apply to self closing elements). |
| Arrow Function Parentheses | yes | Arrow functions can omit parentheses when they have exactly one parameter. In all other cases the parameter(s) must be wrapped in parentheses. This rule enforces the consistent use of parentheses in arrow functions. [[source](https://eslint.org/docs/latest/rules/arrow-parens)] |
| Single Attribute Per Line | no | Enforce single attribute per line in HTML |

### Formatters

| Formatter | configuration file | Homepage | Editor integration |
| --- | --- | --- | --- |
| Prettier | [`.prettierrc`](../.prettierrc) | [https://prettier.io/](https://prettier.io/) | [VSCode](https://marketplace.visualstudio.com/items?itemName=esbenp.prettier-vscode), [WebStorm](https://plugins.jetbrains.com/plugin/10456-prettier), [Sublime Text](https://packagecontrol.io/packages/JsPrettier) and lot of others... check the [official website](https://prettier.io/docs/en/editors.html)|

## Naming conventions

| Object | Convention |
| --- | --- |
| Variables | `camel case` with lowercase first letter, e.g. `familyName` |
| Booleans | We should use `is` or `has` as prefixes, e.g. `isDead`, `hasWife` |
| Functions | Same as with variables + should use descriptive nouns and verbs as prefixes, e.g. `getName()`, `generateReport` |
| Constants | Should be written in `upper snake case`, e.g. `DEFAULT_COUNT_OF_GENERATIONS` |
| Classes | Should be written in `Pascal case`, e.g. `WikiTreePerson` |
| Methods | Same as functions |
| Files | File names must be all lowercase and may include underscores (_) or dashes (-), but no additional punctuation. Follow the convention that your project uses. [[source](https://google.github.io/styleguide/jsguide.html#file-name)] - most Unix-based servers are case sensitives, but this doesn't apply for windows servers.|
MichalVasut marked this conversation as resolved.
Show resolved Hide resolved

### Legend

* `camel case` - a way to separate the words in a phrase by making the first letter of each word capitalized and not using spaces, e.g. `familyName`, `LastName`, ...
* `upper snake case` - words are written in uppercase and separated with underscores, e.g. `DEFAULT_COUNT_OF_GENERATIONS`
* `Pascal case` - same as `camel case`, but the first letter is capitalized, e.g. `WikiTreePerson`

### Conflicting names

#### Problem

> I noticed that both the FanChart view and the Ahnentafal view had an "Ahnentafel" class. I was concerned about there being more than one window.Ahnentafel. Both views seemed to work, but I was seeing some odd problems during integration into WikiTree.com and thought it wouldn't hurt to change this. I modified the class name in the Ahnentafel view (to be "AhnentafelAncestorList").

#### Solutions
1. Agree on some naming conventintions that would prevent this, e.g. use of prefix
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs discussion and the agreement on how to solve this...

1. use `javascript modules` ([docs](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Modules) & [Can I Use page](https://caniuse.com/?search=modules)) and expose to outer world only what is needed.

### Useful reading

* inspired by: [10 JavaScript Naming Conventions Every Developer Should Know](https://www.syncfusion.com/blogs/post/10-javascript-naming-conventions-every-developer-should-know.aspx)

4 changes: 4 additions & 0 deletions contributing.md → docs/contributing.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@ To create your new view, you may want to start with a separate GitHub branch. Th

````git checkout -b newView````

### Check the WikiTree codestyle

Before you start check [WikiTree codestyle](/docs/codestyle.md) and optionally set your editor to follow our formatting rules. The life will be much easier for everybody, especially when checking diffs of your changes at Github ;-).

### Create some new files for your project

1. Create a new subdirectory (e.g. `views/newView/`) inside `views/` to hold the new code
Expand Down
2 changes: 1 addition & 1 deletion index.html
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
<link rel="stylesheet" href="https://www.wikitree.com/css/skeleton.css" />
<link rel="stylesheet" href="https://www.wikitree.com/css/skeleton-wide.css" />
<link rel="stylesheet" href="https://www.wikitree.com/css/layout.css" />
<link href="https://unpkg.com/vis-timeline@latest/styles/vis-timeline-graph2d.min.css">
<link href="https://unpkg.com/vis-timeline@latest/styles/vis-timeline-graph2d.min.css" />

<script src="views/baseDynamicTree/WikiTreeDynamicTreeViewer.js"></script>
<script src="views/restyledBaseExample/restyledBaseExample.js"></script>
Expand Down