software-jessies-org / jessies

Terminator, Evergreen, lwm and friends
GNU General Public License v2.0
84 stars 19 forks source link

Updates Evergreen's file handling to use NIO. This is quite a big change, so I wouldn't mind a second pair of eyes. #40

Closed philjessies closed 3 years ago

philjessies commented 3 years ago

Thanks - very good comments. I'm a bit embarrassed about the string comparison one, to be honest. I blame years of C++ and Go.

Actually, I should re-base (there's been some work on head), and re-do the pull request, as I've also made some fixes since I originally made it.

On Sun, 6 Dec 2020 at 01:53, ritschwumm notifications@github.com wrote:

@ritschwumm commented on this pull request.

In salma-hayek/src/e/util/ProcessUtilities.java https://github.com/software-jessies-org/jessies/pull/40#discussion_r536924426 :

     List<String> command = Arrays.asList(commandArray);

int status = 1; // FIXME: should we signal "something went wrong" more distinctively? try { final Process p = exec(command, directory); if (processListener != null) { processListener.processStarted(p); }

  • Thread inputWriterThread = new Thread(new Runnable() {
  • public void run() {
  • try {
  • BufferedWriter out = new BufferedWriter(new OutputStreamWriter(p.getOutputStream(), "UTF-8"));
  • out.append(input);
  • out.flush();
  • out.close();
  • } catch (Exception ex) {
  • feedExceptionToLineListener(errorLineListener, ex);
  • }
  • Thread inputWriterThread = new Thread(() -> {
  • try {
  • BufferedWriter out = new BufferedWriter(new OutputStreamWriter(p.getOutputStream(), "UTF-8"));

should probably use try-with-resources to ensure the OutputStream is closed even when an exception is encountered

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/software-jessies-org/jessies/pull/40#pullrequestreview-545660830, or unsubscribe https://github.com/notifications/unsubscribe-auth/AE6TQTPJ3UKGX22FSEMKFJDSTLIYJANCNFSM4Q4N4CKQ .