Check for WS problems only on modified lines

Jesse Glick avatarJesse Glick created an issue

The use of fctx.data().splitlines() means that the precommit hook will prevent you from making e.g. a one-line typo fix in a file which already had trailing whitespace before. Since it is not really acceptable in a project with a long history to mix formatting changes with bug fixes (lest you prevent hg annotate from working, introduce spurious merge conflicts, etc.), cleaning up unrelated lines in the file is not an option.

There should be an option to only scan those lines which form part of the diff. For a new file, all lines would be scanned.

Comments (6)

  1. Jesse Glick

    I don't see any purpose (or possibility) for toggling check_diff from the command line, since it only applies to the check hook, which is not associated with any command.

  2. Marcus Lindblom Sonestedt

    There is a checkfiles command too. With the --fixup option it does what the precommit hook does, for all modified files in the working directory.

    The purpose is simple. Make sure every whole file you commit is ws-clean. This makes the process of "fixing" a mixed whitespace repo faster, but it's a matter of taste whether you want it or not (as you refer to in your commit comments).

    What I'm getting at, is that the check_diff option would be nice to have for the command version too. That would either check the current diff (repo[None] context) or all files in repo, instead of doing what the precommit did, checking the full modified files.

    Am I making things clearer?

  3. Jesse Glick

    Ah, you were asking for "diff mode" to be supported for the precommit hook and fixup command. Currently it is not, so adding an option that does the same thing as --config checkfiles.check_diffs=1 (my former interpretation of your request) would be useless.

    Would be nice; I just didn't have time for it yet, since I did not plan on using --fixup and this would mean changes to a different code path. (My text editor, NetBeans, normally deletes trailing whitespace on all modified lines anyway; I just wanted this to be double-checked by a hook.)

  4. Marcus Lindblom Sonestedt

    Precisely. I use a various set of editors, and notepad2 (which I use for .bat-files and simple python fixes) doesn't autoremove, hence my need for precommit/fixup.

    Well, since you've done half the work I can probably take it the rest of the way. ;) I've added this as issue #4 so I don't forget it.

  5. Log in to comment
Tip: Filter by directory path e.g. /media app.js to search for public/media/app.js.
Tip: Use camelCasing e.g. ProjME to search for ProjectModifiedEvent.java.
Tip: Filter by extension type e.g. /repo .js to search for all .js files in the /repo directory.
Tip: Separate your search with spaces e.g. /ssh pom.xml to search for src/ssh/pom.xml.
Tip: Use ↑ and ↓ arrow keys to navigate and return to view the file.
Tip: You can also navigate files with Ctrl+j (next) and Ctrl+k (previous) and view the file with Ctrl+o.
Tip: You can also navigate files with Alt+j (next) and Alt+k (previous) and view the file with Alt+o.