Skip to content
This repository has been archived by the owner on Feb 1, 2019. It is now read-only.

diagnostic.py: Correct row and column number convention #39

Merged
merged 1 commit into from
Apr 5, 2018

Conversation

li-boxuan
Copy link
Member

Row and column number should be zero-based integer, not null

Fixes #37

@gaocegege
Copy link
Member

@jayvdb Could you please have a look why gitmate does not add labels automatically?

@gaocegege gaocegege self-assigned this Mar 31, 2018
@li-boxuan
Copy link
Member Author

Whoops, I ran coala on my laptop and found this commit doesn't pass GitCommitBear check. I'll fix it.

@li-boxuan li-boxuan force-pushed the dev branch 2 times, most recently from e3ae164 to 1f12f0a Compare April 1, 2018 05:56
'line': code['end']['line'] - 1,
'character': code['end']['column']
'line': end_line,
'character': end_char
Copy link
Member

Choose a reason for hiding this comment

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

Should we set the column to 0 or the whole line if the bear returns None?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh yes, I should be careful about that.
According to LSP specification, if there is an issue with whole line x, the output format should be as follows:

{
    start: { line: x, character: 0 }
    end : { line: x + 1, character : 0 }
}

Copy link
Member

Choose a reason for hiding this comment

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

Cool, thus I think we should check if endline == startline.

@li-boxuan li-boxuan force-pushed the dev branch 4 times, most recently from c7e9a6c to 1c094c9 Compare April 4, 2018 03:24
@gaocegege
Copy link
Member

ack 1c094c9

@gaocegege
Copy link
Member

@gitmate-bot rebase

@gitmate-bot
Copy link

Hey! I'm GitMate.io! This pull request is being rebased automatically. Please DO NOT push while rebase is in progress or your changes would be lost permanently ⚠️

@gitmate-bot
Copy link

Automated rebase with GitMate.io was successful! 🎉

@gaocegege
Copy link
Member

@gitmate-bot ff

@gitmate-bot
Copy link

Hey @gaocegege, you do not have the access to perform the fastforward action with GitMate.io. Please ask a maintainer to give you access. ⚠️

@gaocegege
Copy link
Member

@jayvdb Could you please give me access to merge PRs in this repo or have a look at this PR since it blocks #38

@jayvdb
Copy link
Member

jayvdb commented Apr 5, 2018

since it blocks #38

fwiw, PRs can be based on top of another PR.

Row and column number should be zero-based integer, not null

Fixes coala#37
@jayvdb
Copy link
Member

jayvdb commented Apr 5, 2018

@gitmate-bot rebase

@gitmate-bot
Copy link

Hey! I'm GitMate.io! This pull request is being rebased automatically. Please DO NOT push while rebase is in progress or your changes would be lost permanently ⚠️

@gitmate-bot
Copy link

Automated rebase with GitMate.io was successful! 🎉

@jayvdb
Copy link
Member

jayvdb commented Apr 5, 2018

@gitmate-bot ff

@gitmate-bot
Copy link

Hey! I'm GitMate.io! This pull request is being fastforwarded automatically. Please DO NOT push while fastforward is in progress or your changes would be lost permanently ⚠️

@gitmate-bot
Copy link

Automated fastforward with GitMate.io was successful! 🎉

@gitmate-bot gitmate-bot merged commit 7e29998 into coala:master Apr 5, 2018
@li-boxuan li-boxuan deleted the dev branch April 7, 2018 00:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging this pull request may close these issues.

4 participants