Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support split view #13

Open
jzaefferer opened this issue Sep 8, 2014 · 12 comments
Open

Support split view #13

jzaefferer opened this issue Sep 8, 2014 · 12 comments

Comments

@jzaefferer
Copy link
Contributor

Like the new one GitHub has.

@jzaefferer
Copy link
Contributor Author

Is that reasonable at all?

@scottgonzalez
Copy link
Owner

Yeah, it shouldn't be too hard to implement. Just need some time to work on it.

@scottgonzalez
Copy link
Owner

Another early Christmas present for you...

I just pushed a new branch (split), which has an initial implementation for this. The code is ugly as hell, but it seems to work. The branch version just changes all diffs to be split diffs, but the real version will have an option. Play with it and see if it works for various diffs. If everything looks good, I'll get this into master with the option.

@jzaefferer
Copy link
Contributor Author

Nice. Looks promising!

Looking at jquery/jquery-ui@b6bec79 through pretty-diff, the first difference doesn't line up, maybe due to the lines being too long? The first line on both sides wraps on my screen. This is a real problem.

Looking at qunitjs/qunit@d86d6c3 there's a blank/pale line between each of the lines on the right for the new setUrl function. On Github, the background is just solid green, no lines in between. This is okay, though it should be just a CSS issue.

Otherwise I noticed the "file name diff", like this:

--- a/reporter/html.js
+++ b/reporter/html.js

Except now they're side-by-side. They were there before, so not really part of this issue. Could be optimized to show renames when they occur and hide this otherwise.

@scottgonzalez
Copy link
Owner

Just pushed some updates. I've been thinking about changing the handling for the file names for a while. I'll tackle that separately though. Let me know if you find any problems with it now.

@jzaefferer
Copy link
Contributor Author

I ran npm install -g scottgonzalez/pretty-diff#split to install this again.

For this commit, I think the output is worse then before. The last change doesn't line up. That also happens for the other commits I mentioned before:

...

@scottgonzalez
Copy link
Owner

Yeah, I broke the detection of the first line in the lookahead, so everything was off. I also just realized why GitHub is using tables for split view; it's needed to handle one side of the diff having a long line which wraps when the other side doesn't. I'll get that updated soon too.

@jzaefferer
Copy link
Contributor Author

Looks better now. Let me know when you got this tabled, and I'll test again.

Regarding configuration: I like GitHub's model of remember the choice and using that for all diffs until I pick the other view. Maybe you can do the same on the CLI? Once I pick split view using an option, make that the default until I switch again. Like --split and --unified.

@tauren
Copy link

tauren commented Jan 11, 2015

This is great stuff, nice work!

I have a use for both your split branch and your word-diff branch. Will they eventually be usable together and published to NPM? I can make due for now, just interested in your thoughts on the project's roadmap.

@scottgonzalez
Copy link
Owner

Will they eventually be usable together and published to NPM?

I'm not sure that word-diff will ever be usable. If it is, then they'll work together.

@jzaefferer
Copy link
Contributor Author

Btw. I've been using the split view since January or so, haven't noticed any other issues.

@jzaefferer
Copy link
Contributor Author

I don't know where my previous comment came from, I haven't actually used the split view locally...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants