Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
"react-redux": "^5.0.6",
"react-scripts": "1.0.17",
"react-split-pane": "^0.1.74",
"react-toggle": "^4.0.2",
"redux": "^3.7.2",
"redux-devtools-extension": "^2.13.2",
"redux-little-router": "^14.2.3",
Expand Down
10 changes: 7 additions & 3 deletions server/handler/file_pairs.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,21 +33,25 @@ func GetFilePairDetails(repo *repository.FilePairs, diff *service.Diff) RequestP
return nil, serializer.NewHTTPError(http.StatusNotFound, "no file-pair found")
}

var preprocessors []service.DiffPreprocessorFunc

if r.URL.Query().Get("showInvisible") == "1" {
preprocessors = append(preprocessors, service.ReplaceInvisible)
}

diffString, err := diff.Generate(
filePair.Left.Path,
filePair.Right.Path,
filePair.Left.Content,
filePair.Right.Content,
preprocessors...,
)
if err != nil {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return nil, err
}

leftLOC := len(strings.Split(filePair.Left.Content, "\n"))
rightLOC := len(strings.Split(filePair.Right.Content, "\n"))
if err != nil {
return nil, err
}

return serializer.NewFilePairResponse(filePair, diffString, leftLOC, rightLOC), nil
}
Expand Down
28 changes: 26 additions & 2 deletions server/service/diff.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
package service

import "github.com/pmezard/go-difflib/difflib"
import (
"strings"

"github.com/pmezard/go-difflib/difflib"
)

// Diff service generates diff for files
type Diff struct {
Expand All @@ -12,8 +16,20 @@ func NewDiff() *Diff {
return &Diff{context: 6} // keep it hard coded for now
}

// DiffPreprocessorFunc type is function signature to preprocess diffs
type DiffPreprocessorFunc func(string) string

// Generate return unified diff string for 2 files
func (d *Diff) Generate(nameA, nameB, contentA, contentB string) (string, error) {
func (d *Diff) Generate(nameA, nameB, contentA, contentB string, preprocessors ...DiffPreprocessorFunc) (string, error) {
for _, p := range preprocessors {
contentA = p(contentA)
contentB = p(contentB)
}

return d.generate(nameA, nameB, contentA, contentB)
}

func (d *Diff) generate(nameA, nameB, contentA, contentB string) (string, error) {
diff := difflib.UnifiedDiff{
A: difflib.SplitLines(contentA),
B: difflib.SplitLines(contentB),
Expand All @@ -24,3 +40,11 @@ func (d *Diff) Generate(nameA, nameB, contentA, contentB string) (string, error)

return difflib.GetUnifiedDiffString(diff)
}

// ReplaceInvisible preprocessor function that replace invisible character with visible onces
func ReplaceInvisible(content string) string {
content = strings.Replace(content, " ", "·", -1)
content = strings.Replace(content, "\t", "→", -1)
Copy link
Copy Markdown
Contributor

@carlosms carlosms Mar 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about replacing tab with '→ ' to try to keep indentation? I think a single char indentation may make code less readable.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm used to seeing only one → symbol.
If you want more space, we could use ⟶ (⟶ or ⟶).
but whatever you prefer.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's odd, I kind of expect any editor to show the symbol and indent. Like VS Code for example:
tabs

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I definitely saw only → many times.

content = strings.Replace(content, "\r", "^M", -1)
return strings.Replace(content, "\n", "↵\n", -1)
}
14 changes: 14 additions & 0 deletions server/service/diff_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,20 @@ func (suite *DiffSuite) TestDiff() {
assert.Equal(ac, acDiff)
}

func (suite *DiffSuite) TestDiffReplaceInvisible() {
assert := suite.Assert()
inputStr := "line\r\n" +
"\ttabbed line\n" +
" spaced line\n"

assert.Equal(
service.ReplaceInvisible(inputStr),
"line^M↵\n"+
"→tabbed·line↵\n"+
"····spaced·line↵\n",
)
}

func readFile(filename string) (string, error) {
data, err := ioutil.ReadFile(filename)
if err != nil {
Expand Down
10 changes: 8 additions & 2 deletions src/api/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,14 @@ function getAssignments(experimentId) {
return apiCall(`/api/experiments/${experimentId}/assignments`);
}

function getFilePair(experimentId, pairId) {
return apiCall(`/api/experiments/${experimentId}/file-pairs/${pairId}`);
function getFilePair(experimentId, pairId, showInvisible = false) {
let queryStr = '';
if (showInvisible) {
queryStr = '?showInvisible=1';
}
return apiCall(
`/api/experiments/${experimentId}/file-pairs/${pairId}${queryStr}`
);
}

function putAnswer(experimentId, assignmentId, answer) {
Expand Down
33 changes: 25 additions & 8 deletions src/components/Experiment/Diff.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import React, { PureComponent } from 'react';
import { Diff2Html } from 'diff2html';
import Toggle from 'react-toggle';
import 'diff2html/dist/diff2html.css';
import 'react-toggle/style.css';
import './Diff.less';

class Diff extends PureComponent {
Expand All @@ -18,8 +20,14 @@ class Diff extends PureComponent {
}

render() {
const { diffString, leftLoc, rightLoc, className } = this.props;
const showLoc = leftLoc && rightLoc;
const {
diffString,
leftLoc,
rightLoc,
showInvisible,
toggleInvisible,
className,
} = this.props;
const diffHTML = Diff2Html.getPrettyHtml(diffString, {
inputFormat: 'diff',
outputFormat: 'side-by-side',
Expand All @@ -30,14 +38,23 @@ class Diff extends PureComponent {
});
return (
<div className={`diff ${className}`}>
{showLoc && (
<div className="diff__locs">
<div className="diff__loc left">{leftLoc} lines of code</div>
<div className="diff__loc right">{rightLoc} lines of code</div>
<div className="diff__top">
<div className="diff__switch">
<Toggle
checked={showInvisible}
icons={false}
onChange={toggleInvisible}
id="showInvisible"
/>
<label htmlFor="showInvisible">
Show new lines, tabs and spaces
</label>
</div>
)}
<div className="diff__loc left">{leftLoc} lines of code</div>
<div className="diff__loc right">{rightLoc} lines of code</div>
</div>
<div
className={`diff__content ${showLoc ? '_with-loc' : ''}`}
className="diff__content"
dangerouslySetInnerHTML={{ __html: diffHTML }}
/>
</div>
Expand Down
29 changes: 24 additions & 5 deletions src/components/Experiment/Diff.less
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,9 @@
// works in FF but doesn't work in Chrome now
position: relative;

&__locs {
margin-top: 3px;

&__top {
position: absolute;
top: 0;
left: 0;
Expand All @@ -40,16 +42,33 @@
flex: 0;
}

&__switch {
width: 25%;

.react-toggle {
margin-top: -3px;
margin-right: 5px;
}

label {
vertical-align: top;
font-weight: normal;
}
}

&__loc {
width: 50%;
width: 25%;
text-align: right;
padding-right: 10px;
}

&__loc.right {
width: 50%;
flex: 1 0;
}

&__content {
padding-top: 24px;
height: 100%;
}
&__content._with-loc {
padding-top: 20px;
}
}
11 changes: 9 additions & 2 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { composeWithDevTools } from 'redux-devtools-extension';
import thunk from 'redux-thunk';
import rootReducer, { middlewares as stateMiddlewares } from './state';
import { enhancer as routerEnhancer } from './state/routes';
import { logIn } from './state/user';
import { logIn, loadOptions } from './state/user';
import TokenService from './services/token';
import { load as loadGA, middleware as gaMiddleware } from './services/ga';
import './override.less';
Expand All @@ -23,9 +23,16 @@ if (window.cat.GA_TRACKING_ID) {
middlewares.push(gaMiddleware);
}

const initialState = window.cat.initialState || {};
const store = createStore(
rootReducer,
window.cat.initialState || {},
{
...initialState,
user: {
...initialState.user,
...loadOptions(),
},
},
composeWithDevTools(compose(routerEnhancer, applyMiddleware(...middlewares)))
);

Expand Down
22 changes: 17 additions & 5 deletions src/pages/Experiment.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import {
getProgressPercent,
getCurrentFilePair,
} from '../state/assignments';
import { toggleInvisible } from '../state/user';
import { loadFilePair } from '../state/filePairs';
import { makeUrl } from '../state/routes';
import './Experiment.less';

Expand Down Expand Up @@ -74,7 +76,7 @@ class Experiment extends Component {
leftLoc,
rightLoc,
assignmentsOptions,
currentAssigmentId,
currentAssigment,
selectAssigmentId,
markSimilar,
markMaybe,
Expand All @@ -96,10 +98,14 @@ class Experiment extends Component {
<Row className="ex-page__content" onMouseEnter={this.props.hideHeader}>
<Col xs={12} className="ex-page__diff-col">
<Diff
className="ex-page__diff"
diffString={diffString}
leftLoc={leftLoc}
rightLoc={rightLoc}
className="ex-page__diff"
showInvisible={this.props.showInvisible}
toggleInvisible={() =>
this.props.toggleInvisible(expId, currentAssigment.pairId)
}
/>
</Col>
</Row>
Expand All @@ -108,7 +114,7 @@ class Experiment extends Component {
<Selector
title="Previous"
options={assignmentsOptions}
value={currentAssigmentId}
value={currentAssigment.id}
onChange={e => selectAssigmentId(expId, e.target.value)}
/>
</Col>
Expand All @@ -129,9 +135,10 @@ class Experiment extends Component {
}

const mapStateToProps = state => {
const { experiment, assignments } = state;
const { experiment, assignments, user } = state;
const { error, loading, id, name, description } = experiment;
const { list, currentAssigment } = assignments;
const { showInvisible } = user;

const filePair = getCurrentFilePair(state);
const diff = filePair ? filePair.diff : null;
Expand All @@ -151,8 +158,9 @@ const mapStateToProps = state => {
diffString: diff,
leftLoc: filePair ? filePair.leftLoc : 0,
rightLoc: filePair ? filePair.rightLoc : 0,
currentAssigmentId: currentAssigment ? currentAssigment.id : null,
currentAssigment,
assignmentsOptions,
showInvisible,
};
};

Expand All @@ -164,6 +172,10 @@ const mapDispatchToProps = dispatch => ({
selectAssigmentId: (expId, id) =>
dispatch(push(makeUrl('question', { experiment: expId, question: id }))),
finish: expId => dispatch(push(makeUrl('finish', { experiment: expId }))),
toggleInvisible: (expId, id) => {
dispatch(toggleInvisible());
return dispatch(loadFilePair(expId, id));
},
});

export default connect(mapStateToProps, mapDispatchToProps)(
Expand Down
Loading