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

Regression: startColumn makes CSV parser return empty array. #7272

Closed
TorsteinHonsi opened this Issue Oct 13, 2017 · 4 comments

Comments

Projects
None yet
2 participants
@TorsteinHonsi
Collaborator

TorsteinHonsi commented Oct 13, 2017

Live demo with steps to reproduce

http://jsfiddle.net/hr6j17eq/2/

@TorsteinHonsi

This comment has been minimized.

Show comment
Hide comment
@TorsteinHonsi

TorsteinHonsi Oct 13, 2017

Collaborator

Related: #7265

Collaborator

TorsteinHonsi commented Oct 13, 2017

Related: #7265

@TorsteinHonsi

This comment has been minimized.

Show comment
Hide comment
@TorsteinHonsi

TorsteinHonsi Oct 16, 2017

Collaborator

There's still something fishy - check out this test. It concatenates column 0 with column 1. This test can replace the last test in data/csv/demo.js when fixed.

QUnit.test('startRow, endRow, startColumn, endColumn', function (assert) {
    var data =
        'Pad,Pad,Pad,Pad\n' +
        'Pad,Apples,Pears,Pad\n' +
        'Pad,1,2,Pad\n' +
        'Pad,5,6,Pad\n' +
        'Pad,Pad,Pad,Pad';

    Highcharts.data({
        csv: data,
        startRow: 1,
        endRow: 3,
        startColumn: 1,
        endColumn: 2,
        parsed: function () {

            assert.strictEqual(
                this.columns.length,
                2,
                'Two columns included'
            );
            assert.strictEqual(
                this.columns[0].join(','),
                'Apples,1,5',
                'First column ok'
            );
            assert.strictEqual(
                this.columns[0].length,
                3,
                'Three rows included'
            );
        }
    });

});
Collaborator

TorsteinHonsi commented Oct 16, 2017

There's still something fishy - check out this test. It concatenates column 0 with column 1. This test can replace the last test in data/csv/demo.js when fixed.

QUnit.test('startRow, endRow, startColumn, endColumn', function (assert) {
    var data =
        'Pad,Pad,Pad,Pad\n' +
        'Pad,Apples,Pears,Pad\n' +
        'Pad,1,2,Pad\n' +
        'Pad,5,6,Pad\n' +
        'Pad,Pad,Pad,Pad';

    Highcharts.data({
        csv: data,
        startRow: 1,
        endRow: 3,
        startColumn: 1,
        endColumn: 2,
        parsed: function () {

            assert.strictEqual(
                this.columns.length,
                2,
                'Two columns included'
            );
            assert.strictEqual(
                this.columns[0].join(','),
                'Apples,1,5',
                'First column ok'
            );
            assert.strictEqual(
                this.columns[0].length,
                3,
                'Three rows included'
            );
        }
    });

});
@cvasseng

This comment has been minimized.

Show comment
Hide comment
@cvasseng

cvasseng Oct 16, 2017

Contributor

This may be related to the fix I did for the delimiter guessing, if so I have a hunch on how to fix it. WIll try to take a look today.

Contributor

cvasseng commented Oct 16, 2017

This may be related to the fix I did for the delimiter guessing, if so I have a hunch on how to fix it. WIll try to take a look today.

@cvasseng

This comment has been minimized.

Show comment
Hide comment
@cvasseng

cvasseng Oct 17, 2017

Contributor

Shouldn't we keep the last test in data/csv/demo.js? The current last one tests string quoting (e.g. "A string with ""quotes"" in it", while the above one tests start/end row/column.

Contributor

cvasseng commented Oct 17, 2017

Shouldn't we keep the last test in data/csv/demo.js? The current last one tests string quoting (e.g. "A string with ""quotes"" in it", while the above one tests start/end row/column.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment