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

Wrong display column #134

Closed
ncannasse opened this issue Jun 22, 2017 · 13 comments
Closed

Wrong display column #134

ncannasse opened this issue Jun 22, 2017 · 13 comments
Assignees
Labels

Comments

@ncannasse
Copy link
Member

When using tabs, if I'm getting an error "line 918, characters 2-5", the red tagline is showing wrong place:

image

@ncannasse ncannasse changed the title Wrong display with tabs Wrong display column Jun 23, 2017
@ncannasse
Copy link
Member Author

PS: this has nothing to do with tabs, it's because Haxe uses 0-based column index while VSCode requires 1-based. Will keep open until I find a way to deal with it in haxe

@nadako
Copy link
Member

nadako commented Jun 23, 2017

Note that this was/is also a problem Sublime Text problem matchers that expect 1-based columns. And it might be a good idea to change the reported column to 1-based in Haxe 4, so it's consistent with 1-based line numbers. Since Haxe 4 will already report different column numbers in some situations (because of unicode lexer), I think it would be fair to change that as well without worrying about backward compatibility.

OTOH, this is also an issue with OCaml vscode extensions, and if someday they provide a way to process the output with an extension before reporting problem, we might as well just handle the 0-based columns.

@ncannasse
Copy link
Member Author

I don't think we should change anything in haxe "without worrying about backward compatibility" :)
I'll add a switch to the compiler for now, we'll see how it goes

@nadako
Copy link
Member

nadako commented Jun 23, 2017

well, it's not like we ever provided any proper api for the positions to break, it's just an arbitrary string...

@ncannasse
Copy link
Member Author

All existing Haxe IDE support is based on 0-based output so we can't change it without breaking them

@nadako
Copy link
Member

nadako commented Jun 23, 2017

Alright, Haxe 4 now uses 1-based column numbers. I'll have to remove the +1s from a couple places in haxe-language-server.

@back2dos
Copy link

back2dos commented Jun 24, 2017

Alright, Haxe 4 now uses 1-based column numbers. I'll have to remove the +1s from a couple places in haxe-language-server.

This would have to be version aware. I suggest to use -D old-error-format instead, as that seems a bit less complicated.

@Gama11
Copy link
Member

Gama11 commented Jun 24, 2017

Version-awareness shouldn't be much of an issue, we already check the Haxe version in a few places in haxe-languageserver.

@Simn
Copy link
Member

Simn commented Jun 24, 2017

I suppose the point is that we shouldn't remove that handling.

@back2dos
Copy link

It's always possible to add complexity, but rarely wise ;)

@back2dos
Copy link

Also: what Simn said.

@Gama11
Copy link
Member

Gama11 commented Jun 24, 2017

I suppose the point is that we shouldn't remove that handling.

Well, yeah, we still have to support Haxe 3.4.x - I thought that was a given?

@nadako
Copy link
Member

nadako commented Jun 24, 2017

Of course we're not breaking haxe 3.x support with this, but since we already have a system for handling positions differently depending on version (because of utf8), we might as well improve it a bit for the sake of cleanliness. But anyway I'll have to look into whether it's worth it compared to using the define, yeah.

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

No branches or pull requests

5 participants