privacytools / privacytools.io

🛡🛠 You are being watched. Protect your privacy against global mass surveillance.
https://www.privacyguides.org
Creative Commons Zero v1.0 Universal
3.11k stars 388 forks source link

💬 Discussion | Use code formatter #1093

Open dawidpotocki opened 4 years ago

dawidpotocki commented 4 years ago

Related: https://github.com/privacytoolsIO/privacytools.io/pull/1089

Problem

There are many inconsistencies and code is hard to read.

_includes/sections/vpn.html uses 2 spaces for indentation _includes/panel.html uses 4 spaces for indentation in some places we can see 3 spaces used by mistake

Many <> HTML tags are used next to each other, instead of under.

<li><a href="https://quad9.net/">Quad9 DNS</a> - A non-profit, anycast DNS provider founded by <a href="https://www-03.ibm.com/press/us/en/pressrelease/53388.wss">IBM</a>, <a href="https://www.pch.net/">PCH</a>, and <a href="https://www.globalcyberalliance.org/quad9/">Global Cyber Alliance</a>. Provides malicious domain filtering and supports DoH, DoT, and DNSCrypt. <span class="badge badge-warning" data-toggle="tooltip" title="Founders of Global Cyber Alliance include: City of London Police & Manhattan District Attorney's Office">Warnings <i class="far fa-question-circle"></i></span></li>

instead of

<li>
  <a href="https://quad9.net/">Quad9 DNS</a>
  - A non-profit, anycast DNS provider founded by
  <a href="https://www-03.ibm.com/press/us/en/pressrelease/53388.wss">IBM</a>,
  <a href="https://www.pch.net/">PCH</a>, and
  <a href="https://www.globalcyberalliance.org/quad9/">Global Cyber Alliance</a>.
  Provides malicious domain filtering and supports DoH, DoT, and DNSCrypt.
  <span class="badge badge-warning" data-toggle="tooltip" title="Founders of Global Cyber Alliance include: City of London Police & Manhattan District Attorney's Office">
    Warnings
    <i class="far fa-question-circle"></i>
  </span>
</li>

or this

<li>
  <a href="https://quad9.net/">Quad9 DNS</a> - A non-profit, anycast DNS provider
  founded by
  <a href="https://www-03.ibm.com/press/us/en/pressrelease/53388.wss">IBM</a>,
  <a href="https://www.pch.net/">PCH</a>, and
  <a href="https://www.globalcyberalliance.org/quad9/">Global Cyber Alliance</a>.
  Provides malicious domain filtering and supports DoH, DoT, and DNSCrypt.
  <span
    class="badge badge-warning"
    data-toggle="tooltip"
    title="Founders of Global Cyber Alliance include: City of London Police & Manhattan District Attorney's Office"
  >
    Warnings
    <i class="far fa-question-circle"></i>
  </span>
</li>

Problem with giving everything on one line is readability. There is a reason why books don't have enormous pages. It is easier and faster to not move eyes so much. Also on smaller screens it is hard to read it, as either we get a scrollbar or it overflows to next line.

Solution

Using code formatter. It would make code more readable, consistent and easier to maintain. Formatting could be check by Travis, so code would always be formatted properly.

prettier

It is formatter for HTML/JavaScript/CSS/YAML/Markdown, can be used in CLI and integrated with many of popular editors (Vim, Emacs, VS Code, Web Storm, Sublime Text…)

js-beautify

It is formatter for HTML/JavaScript/CSS. Can also be used in CLI and integrated with code editors, but there are less plugins for them and seem to be worse quality than prettier's.

nitrohorse commented 4 years ago

Also related is https://github.com/privacytoolsIO/privacytools.io/pull/900.

https://github.com/privacytoolsIO/privacytools.io/pull/1089 is now a working MR with pre-commit and Travis integration working :+1: Will do some testing with Prettier to see if I can configure it properly for this repo too. Then we could compare.

jamesponddotco commented 4 years ago

The repository could also have a .editorconfig file, which would help everyone to use the same coding style.

EditorConfig

dngray commented 4 years ago

I would support this as its hard to know what the "standard" should be.

Gusted commented 3 years ago

I would support this as its hard to know what the "standard" should be.

Just want to add my 2 cents into this discussion.

Everyone has their own personal preference for their indents and tabs etc. Therefore standards where created e.g. Google Airbnb standard/standard. Everyone has their own reasons for why x has to be y, so why not follow some basic principles. Linux coding-style has good principles for why x has to be y.

Let's take for example the indents.

Rationale: The whole idea behind indentation is to clearly define where a block of control starts and ends. 
Especially when you’ve been looking at your screen for 20 straight hours, 
you’ll find it a lot easier to see how the indentation works if you have large indentations.

If you take this principle you can throw 2 indents over board and choose for 4 or 8. Ultimately following this principle, however that introduce another problem.

Now, some people will claim that having 8-character indentations makes the code move too far to the right, 
and makes it hard to read on a 80-character terminal screen. 
The answer to that is that if you need more than 3 levels of indentation, 
you’re screwed anyway, and should fix your program.

Well, I disagree with the last sentence as we are mainly speaking about HTML that can have short tags or tags with attributes and can have easily more than 3 levels indentation. Following those 2 simple understandable principles a 4 indent is a good and reasonable choice.

As for the tool using, with considering the main language of this repo is html. prettier seems to have a better fit for this and can be customized with config(.prettierrc), which most IDE's automatically will use and don't need an .editorconfig configured to this.

Regards, Gusted