search

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

Dev #58

Closed ojles closed 7 years ago

ojles commented 7 years ago

I've merged your changes and fixed merge conflicts

nanoandrew4 commented 7 years ago

@Sergix @ojles Just make sure the new Exec class is still in, there has been a bit of confusion with it and it seems to be reverting to the old version here.

nanoandrew4 commented 7 years ago

Also, we should decide whether files should be formatted using tabs or spaces. I started with spaces and have changed all my files to use tab characters, but this commit seems to use spaces to indent, which will mean each time I pull my IDE will complain.

Although the source code styling sheet specifies that a TAB key press should be used, IDE's can interpret that as 4 or 8 spaces, or a tab character. @Sergix care to clarify, so we can keep it consistent going forward?

ojles commented 7 years ago

@nanoandrew4 In Exec#getRest() I changed String to StringBuilder and formatted the code. I don't think we are reverting to the old version.

nanoandrew4 commented 7 years ago

@ojles See the master branch Exec.java for the correct version, the one here I'm pretty sure is older.

Link: https://github.com/Sergix/JTerm/blob/master/src/main/java/jterm/command/Exec.java

ojles commented 7 years ago

@nanoandrew4 Can you tell me the exact line of code that's confusing you? Because I don't see any difference. And btw, I'm pulling my changes into dev, not master

nanoandrew4 commented 7 years ago

@ojles My apologies, I misread the diff, you are correct there is no difference. All is in order in the file. That is the correct Exec.java file.

Sergix commented 7 years ago

So all is well here?

nanoandrew4 commented 7 years ago

@Sergix No, you still need to decide whether files should use 4 spaces or the tab character (since IDE's can do both using the TAB key). Although the docs mention that indentation is done using the TAB key, IDE's can either input four spaces or a tab character. This pull request uses the four spaces for indentation. So it's probably a good idea to pick one now and make it known going forward, so everyone uses the same formatting.

Sergix commented 7 years ago

Well, for most IDE's the TAB key and 4 spaces are the same width. For use, I think we should just go ahead and use 4 spaces from now on. I'll make that change in the Style Guide, as well as everything else that needs to be changed.

nanoandrew4 commented 7 years ago

Sounds good, I'll make the changes in my IDE. I'm pretty sure this pull request is good to go then.