Settle IDE Whitespace Conflicts

[farblos]

What developers really get upset about ...

-- EmacsWiki.

Hi @ojehle, your commit 3a91fbe introduced a lot of whitespace changes, I guess resulting from your IDE reformatting the code to its standards. (Which IDE do you use, BTW?)

Um, I have a couple of concerns with that:

• It completely hides the "real" change you made ("Download all possible files first and then exit if there was a search").

• It makes it very difficult to merge the pending branch "redo-authentication" of mine with rather complex changes, since that still uses the old formatting.

• It undoes some of my formatting which I really like.

Here are the style changes that I see in commit 3a91fbe, ordered from "fine with me" to "please keep these my way":

• Some reordering and cleanup in the imports (fine with me)

• Added one blank after casts (fine withe me)

• Changed braces from this style

  }
  catch (Exception e) {
      warn("Cannot write to request-response log file", e);
  }

  to this style:

  } catch (Exception e) {
      warn("Cannot write to request-response log file", e);
  }

  (fine with me)

• Method definitions rearranged to fit the definition mostly on one long line. Likewise for other code lines.

  Mostly fine with me, but if we could adjust the target line length to a somewhat shorter value (100?) I'd be even finer.

• Normalized non-indentation whitespace. Please keep these my way. I just find this:

  longopts.add( new LongOpt("help",      LongOpt.NO_ARGUMENT,       null, 'h') );
  longopts.add( new LongOpt("debug",     LongOpt.NO_ARGUMENT,       null, 'D') );
  longopts.add( new LongOpt("quiet",     LongOpt.NO_ARGUMENT,       null, 'Q') );
  longopts.add( new LongOpt("directory", LongOpt.REQUIRED_ARGUMENT, null, 'd') );
  longopts.add( new LongOpt("patches",   LongOpt.REQUIRED_ARGUMENT, null, 'x') );
  longopts.add( new LongOpt("patchfile", LongOpt.REQUIRED_ARGUMENT, null, 'f') );

  so much more readable than this:

  longopts.add(new LongOpt("help", LongOpt.NO_ARGUMENT, null, 'h'));
  longopts.add(new LongOpt("debug", LongOpt.NO_ARGUMENT, null, 'D'));
  longopts.add(new LongOpt("quiet", LongOpt.NO_ARGUMENT, null, 'Q'));
  longopts.add(new LongOpt("directory", LongOpt.REQUIRED_ARGUMENT, null, 'd'));
  longopts.add(new LongOpt("patches", LongOpt.REQUIRED_ARGUMENT, null, 'x'));
  longopts.add(new LongOpt("patchfile", LongOpt.REQUIRED_ARGUMENT, null, 'f'));

  At least on Eclipse (if you happen to use that) there seems to be an option to disable formatting for some code regions (@formatter:off|on). I could use that option for the few instances where I'd like to keep column alignment in code.

• Normalized whitespace in comments. Please keep these my way. When I indent or align text in comments I do so very carefully. Just compare this comment block before your commit:

  //   if (page instanceof HtmlPage &&
  //       (hpage = (HtmlPage)page) != null &&
  //       <tests on hpage>) {
  //     [...]
  //     page = <operate on hpage>;
  //     dump(page);
  //   }

  to this one:

  // if (page instanceof HtmlPage &&
  // (hpage = (HtmlPage)page) != null &&
  // <tests on hpage>) {
  // [...]
  // page = <operate on hpage>;
  // dump(page);
  // }

  Funny as it seems, I also need two spaces after a period ending a sentence for my editor. Which is Emacs, BTW.

  And these I think I would not be able to switch of with any @formatter:off|on directives, since there are just too many of them.

Probably we could continue like this:

• I merge the net changes of your both recent commits (integer overflow, error handling with failing searches) without the whitepspace changes into my pending "redo-authentication" branch.

• You force-merge that branch into your master, which effectively would undo all your whitespace changes but keep the others.

• You change your IDE settings such that I will be happier: Set target line length to, say, 100 characters, and do not touch non-indentation whitespace, at least not in comments.

• We do one synchronized commit only with these whitepace changes so that neither you nor me needs to merge whitespace changes now and in future.

What do you think?

[ojehle]

i used eclipse, with all standard settings. but as i know i can import formatter settings there, if you can provide me your settings. my really small change on the logic was only to replace error exit during the search with a warning and set a boolean to indicate there was an error. after downloading all found patches, the boolean is checked and if set, the exit with errorAm 09.05.2024 um 19:53 schrieb farblos @.***>: What developers really get upset about ...

-- EmacsWiki. Hi @ojehle, your commit 3a91fbe introduced a lot of whitespace changes, I guess resulting from your IDE reformatting the code to its standards. (Which IDE do you use, BTW?) Um, I have a couple of concerns with that:

It completely hides the "real" change you made ("Download all possible files first and then exit if there was a search").

It makes it very difficult to merge the pending branch "redo-authentication" of mine with rather complex changes, since that still uses the old formatting.

It undoes some of my formatting which I really like.

Here are the style changes that I see in commit 3a91fbe, ordered from "fine with me" to "please keep these my way":

Some reordering and cleanup in the imports (fine with me)

Added one blank after casts (fine withe me)

Changed braces from this style } catch (Exception e) { warn("Cannot write to request-response log file", e); }

to this style: } catch (Exception e) { warn("Cannot write to request-response log file", e); }

(fine with me)

Method definitions rearranged to fit the definition mostly on one long line. Likewise for other code lines. Mostly fine with me, but if we could adjust the target line length to a somewhat shorter value (100?) I'd be even finer.

Normalized non-indentation whitespace. Please keep these my way. I just find this: longopts.add( new LongOpt("help", LongOpt.NO_ARGUMENT, null, 'h') ); longopts.add( new LongOpt("debug", LongOpt.NO_ARGUMENT, null, 'D') ); longopts.add( new LongOpt("quiet", LongOpt.NO_ARGUMENT, null, 'Q') ); longopts.add( new LongOpt("directory", LongOpt.REQUIRED_ARGUMENT, null, 'd') ); longopts.add( new LongOpt("patches", LongOpt.REQUIRED_ARGUMENT, null, 'x') ); longopts.add( new LongOpt("patchfile", LongOpt.REQUIRED_ARGUMENT, null, 'f') );

so much more readable than this: longopts.add(new LongOpt("help", LongOpt.NO_ARGUMENT, null, 'h')); longopts.add(new LongOpt("debug", LongOpt.NO_ARGUMENT, null, 'D')); longopts.add(new LongOpt("quiet", LongOpt.NO_ARGUMENT, null, 'Q')); longopts.add(new LongOpt("directory", LongOpt.REQUIRED_ARGUMENT, null, 'd')); longopts.add(new LongOpt("patches", LongOpt.REQUIRED_ARGUMENT, null, 'x')); longopts.add(new LongOpt("patchfile", LongOpt.REQUIRED_ARGUMENT, null, 'f'));

At least on Eclipse (if you happen to use that) there seems to be an option to disable formatting for some code regions @.***:off|on). I could use that option for the few instances where I'd like to keep column alignment in code.

Normalized whitespace in comments. Please keep these my way. When I indent or align text in comments I do so very carefully. Just compare this comment block before your commit: // if (page instanceof HtmlPage && // (hpage = (HtmlPage)page) != null && // ) { // [...] // page = ; // dump(page); // }

to this one: // if (page instanceof HtmlPage && // (hpage = (HtmlPage)page) != null && // ) { // [...] // page = ; // dump(page); // }

Funny as it seems, I also need two spaces after a period ending a sentence for my editor. Which is Emacs, BTW. And these I think I would not be able to switch of with any @formatter:off|on directives, since there are just too many of them.

Probably we could continue like this:

I merge the net changes of your both recent commits (integer overflow, error handling with failing searches) without the whitepspace changes into my pending "redo-authentication" branch.

You force-merge that branch into your master, which effectively would undo all your whitespace changes but keep the others.

You change your IDE settings such that I will be happier: Set target line length to, say, 100 characters, and do not touch non-indentation whitespace, at least not in comments.

We do one synchronized commit only with these whitepace changes so that neither you nor me needs to merge whitespace changes now and in future.

What do you think?

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: @.***>

[farblos]

Sounds good. I'll prepare something, but that will take some time.

[farblos]

Closed with PR #15.