Sergix / JTerm

A terminal written in Java for cross-platform compatibility and usage.
https://sergix.github.io/projects/jterm
GNU General Public License v3.0
52 stars 32 forks source link

Reworked initCommands() and some more cleanup #99

Closed marcluque closed 6 years ago

marcluque commented 6 years ago

The initCommands() method now uses a different approach which ensures that the commands are loaded safely on every system.

lbenedetto commented 6 years ago
  1. You readded unused variables in Clear.java
  2. Why do you add random newlines everywhere?
marcluque commented 6 years ago
  1. Indeed, I must have messed up to delete them when I was resolving the merge conflicts, sorry about that.

  2. It's for readability. It's a good practice to e.g. make new lines when you have a new class before something else comes. Same for control structures, to enhance the code and it's readability. The Google Java Style Guide advices to uses spaces and some logical order for readability. That's basically what I did when separating some code. For example you separate instances of objects together with the methods that are called on the object, control structures from not directly related code etc. I did it with system, it might seem a bit random at first, but it's actually quite nice to enhance the code like this^^

See here for more information: https://google.github.io/styleguide/javaguide.html

lbenedetto commented 6 years ago

Yeah, some of them make sense. But having two new lines instead of one before every class declaration just seems unnecessary. No point undoing it either I guess

marcluque commented 6 years ago

Is there a particular case that has those 2 lines before a class declaration? Could you link it for me in that case? Because I would agree on that, it's not necessary.

lbenedetto commented 6 years ago

Ok, so that's the diff thing playing tricks on me. It looks like there are 3 newlines between the class declaration and it's first method, but you actually only added one taking the number of lines from 0 to 1

marcluque commented 6 years ago

Okay nice I'm relieved it was just the diff. thing and the spacings are working as I wanted them to. Thank you even though for the question referring to the spaces and the reminder of the variables. I'll take care of removing them in the next PR ^^