danvk / webdiff Goto Github PK

github does this well. On this PR, for example, it shows the file rename this way:

cycledash/static/js/examine/{examine.js → ExaminePage.js}

This is much better than what webdiff shows at the moment:

cycledash/static/js/examine/examine.js → cycledash/static/js/examine/ExaminePage.js

This can sometimes happen when you reload, or when you run webdiff file1 file2.

Here's an example with github's viewer:
cameronmcefee/Image-Diff-View-Modes@8e95f70#diff-3

they center the two images on top of one another, which seems reasonable.

The error is something like width is undefined.

webdiff testdata/images/{left,right}

The gray background areas don't change, but they appear to in this video:

It's fairly easy to hit GitHub's rate limit with webdiff's pull request feature. There should be an easy way to OAuth in.

Would be a nice option to have

webdiff listens for the onbeforeunload event and kills the web server when it fires. This usually corresponds to closing the web server, so this behavior makes sense. But if you reload, the event also fires and the reload will fail.

A solution might be to wait 0.5 seconds before killing the server.

The tests are failing and I had no idea!

git commit is fairly good about detecting when you move + modify a file. git webdiff detects pure moves, but it should also detect move + modify in a similar way to how git does it. I'd assume the algorithm involves % of identical lines, or % of the file that's covered by identical diffhunks.

"git diff" does a terrible job of this, but it should be easy to detect pure renames in webdiff. Detecting rename + modification is harder, but I'm sure there are good algorithms for this.

If you try to view changes to a very large file (say a 4 MB one line JSON file), the browser will just hang.

Better behavior would be to say something like "large file (4,567,890 bytes)".

You can reload a diff page to see changes when a file changes on disk. But the metadata (e.g. image dimensions, whether there are any changes) does not get updated.

A frequent use case is to run git webdiff to review your unstaged changes in a git repo. When run this way, it would be helpful to have a "stage" button which git adds the file you're looking at.

This is a less ambitious version of #11.

The range slider is a little silly in that case and can be off-screen for large images. It would make sense to allow the red/green dividing line itself to be dragged.

Check out GitX; it has not been updated since 2009 AFAICT but I use it as a pretty handy diff viewer / commit-creation tool. Notable features:

• stage/unstage diffs
  • per file
  • per logical diff chunk
  • per line
• type commit messages / create commits
• bonus: ammend commits
  • somewhat hackily implemented / buggy

GitX has some annoying issues:

• no way to turn off OSX spell-checker by default (always turns on at start up), leading to many DYACs in commit msgs
• sometimes mysteriously spins with 100% CPU in background when it shouldn't be doing anything
• generally kind of slow
• could use some hot-key love

Some of GitX combined with your vision for webdiff could be really killer: better diffing (e.g. inline diff highlighting), syntax highlighting, general web-interface goodness, reflecting changes on disk in realtime, allowing inline editing?!!! so many possibilities.

Would love to port my existing GitX flow (stage/unstage/commit) to webdiff to start, though.

Or even just /2. Now that webdiff allows reload, it should preserve as much of the page as possible when you do so.

If you run webdiff non-existent-file other-file, it will start a server, open up the web browser and then complain. It should instead terminate immediately with a helpful error message.

Here's the error I see:

$ git remote -v
origin  [email protected]:danvk/dygraphs.git (fetch)
origin  [email protected]:danvk/dygraphs.git (push)

$ webdiff #292
Traceback (most recent call last):
  File "/usr/local/bin/webdiff", line 11, in <module>
    sys.exit(run())
  File "/usr/local/lib/python2.7/site-packages/webdiff/app.py", line 228, in run
    parsed_args = argparser.parse(sys.argv[1:])
  File "/usr/local/lib/python2.7/site-packages/webdiff/argparser.py", line 49, in parse
    owner, repo, num = github_fetcher.get_pr_repo(num)
  File "/usr/local/lib/python2.7/site-packages/webdiff/github_fetcher.py", line 77, in get_pr_repo
    remotes = _get_github_remotes()
  File "/usr/local/lib/python2.7/site-packages/webdiff/github_fetcher.py", line 113, in _get_github_remotes
    return [parse(remote.group(3)) for remote in remotes]
  File "/usr/local/lib/python2.7/site-packages/webdiff/github_fetcher.py", line 110, in parse
    assert m, 'Unable to parse github remote %s' % remote
AssertionError: Unable to parse github remote git

Here's an example session:

$ webdiff https://github.com/hammerlab/cycledash/pull/200/files
Loading pull request hammerlab/cycledash#200 from github...
  __tests__/pdifftests/examine-charted.png...
  __tests__/pdifftests/examine-sorted.png...
  __tests__/pdifftests/examine-tooltip.png...
  __tests__/pdifftests/examine.png...
Serving diffs on http://localhost:51234
Close the browser tab or hit Ctrl-C when you're done.
----------------------------------------
Exception happened during processing of request from ('127.0.0.1', 51342)
Traceback (most recent call last):
  File "/usr/local/Cellar/python/2.7.6_1/Frameworks/Python.framework/Versions/2.7/lib/python2.7/SocketServer.py", line 295, in _handle_request_noblock
    self.process_request(request, client_address)
  File "/usr/local/Cellar/python/2.7.6_1/Frameworks/Python.framework/Versions/2.7/lib/python2.7/SocketServer.py", line 321, in process_request
    self.finish_request(request, client_address)
  File "/usr/local/Cellar/python/2.7.6_1/Frameworks/Python.framework/Versions/2.7/lib/python2.7/SocketServer.py", line 334, in finish_request
    self.RequestHandlerClass(request, client_address, self)
  File "/usr/local/Cellar/python/2.7.6_1/Frameworks/Python.framework/Versions/2.7/lib/python2.7/SocketServer.py", line 651, in __init__
    self.finish()
  File "/usr/local/Cellar/python/2.7.6_1/Frameworks/Python.framework/Versions/2.7/lib/python2.7/SocketServer.py", line 710, in finish
    self.wfile.close()
  File "/usr/local/Cellar/python/2.7.6_1/Frameworks/Python.framework/Versions/2.7/lib/python2.7/socket.py", line 279, in close
    self.flush()
  File "/usr/local/Cellar/python/2.7.6_1/Frameworks/Python.framework/Versions/2.7/lib/python2.7/socket.py", line 303, in flush
    self._sock.sendall(view[write_offset:write_offset+buffer_size])
error: [Errno 32] Broken pipe
----------------------------------------

^C%

It's not really clear that you're supposed to bounce on the "b" key to make this happen.

This would make the initial diff load more quickly and reduce the likelihood of exhausting your API request limit.

It might require some refactoring, since this would break the model that webdiff always diffs two directories.

Large diffs can take a few seconds to load. Showing a spinner or some other loading indicator would be a helpful affordance.

Looking at this PR:

1 file added, 49 modified, 67 deleted:

$ gd --name-status $oh..$pr/478 | first | hist 1 A 49 M 67 D

Github PR view is really bogged down by all the contents of the deleted files:

Resulting in me doing things like:

# grep for Modified files and diff them
$ gd $oh..$pr/478 -- $(gd --name-status $oh..$pr/478 | grep '^M' | col 1)

It would be cool if we provided some enhanced capabilities for slicing/dicing PRs, e.g.

• show only names of deleted files
• filter to view to only Modified, Deleted, or Added files

...and it probably should..?

Since browsers are already good at showing images, this shouldn't be too difficult.

For bonus points, show a perceptual diff!

See screenshot, I'm getting a scrollbar on both the container page and the iframe'd page.

The git UI for merges is confusing:

<<<blah
blah
======
blah
>>>blah

It would be nice if webdiff could show the three-way merge.

when inside the submodule directory, running webdiff returns an error:

fatal: Could not switch to '../../../../Submodules/': No such file or directory
diff --raw --no-abbrev -z: command returned error: 128

contents of the .git in the submodule directory is:

../../.git/modules/Submodules/mixpanel

lmk if you need anything else to repro, but i'm pretty sure this is true for any submodule

Rietveld & co show a red ≫ symbol.

Should allow custom tab widths. Default should probably be 8.

Ala github: https://github.com/hammerlab/cycledash/pull/175/files

Updating codediff.js might be sufficient to get this, see danvk/github-syntax#25

This would be simple to add and would make it clear where the big changes are.

github shows a grid pattern, which seems reasonable.

I'm getting an error installing the latest update in yosemite:

running pip install webdiff generates this error:

Command /usr/bin/python -c "import setuptools, tokenize;__file__='/private/tmp/pip_build_root/pillow/setup.py';exec(compile(getattr(tokenize, 'open', open)(__file__).read().replace('\r\n', '\n'), __file__, 'exec'))" install --record /tmp/pip-09Jsha-record/install-record.txt --single-version-externally-managed --compile failed with error code 1 in /private/tmp/pip_build_root/pillow
Storing debug log for failure in /Users/adam/Library/Logs/pip.log

This would shrink the images to fit in the current browser window. Behavior would have to be different for side-by-side (which always shows two images) and the other modes (which use one image's worth of space).

When you switch to "blink" mode for an image diff, it's impossible to tell whether you're looking at the left or right image.

"n" and "p" do this in Critique and other review tools. Would be a nice complement to j/k.

It's purely duplicative in this (fairly common) case:

It's because imageDiffMode is getting set to undefined.

This would let you use webdiff in place of plain-old diff. Currently it requires two directories as parameters, but it would be nice if it could accept two files as well.

This would be valuable, easy to calculate information for image diffs.

github does this in its new image diff UI. It's quite helpful for indicating which image is the before and which is the after, which is a problem with "blink" mode in webdiff now.

Would be nice to get syntax highlighting for JSX (ReactJS) files.

Repro via:

webdiff testdata/binarydiff/{left,right}

non-centered images look goofy w/ the centered tools bar:

The bright shade of green gets used for pure adds. A lighter shade would be easier to look at let the syntax highlighting shine through. The green is not very information-rich in a pure add.

This currently shows up as an empty diff. There should be a message at the top.

If you run webdiff #292 in a github repo, it should diff pull request number 292 for that repo.

For bonus points, webdiff # should bring up a list of PRs that you can pick from.

You can see both the red & green border here, which looks odd:

Probably an off-by-one error, or an issue w/ box-sizing.