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

Go To description mentions line, column but it should be line, character #14598

Closed
alexdima opened this issue Oct 27, 2016 · 12 comments
Closed
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug verified Verification succeeded
Milestone

Comments

@alexdima
Copy link
Member

Extracted from #14585 (comment)

@alexdima alexdima added the bug Issue identified by VS Code Team member as probable bug label Oct 27, 2016
@bpasero
Copy link
Member

bpasero commented Oct 27, 2016

@alexandrudima in the end I am passing a Range to the editor with start line number and start column (https://github.com/Microsoft/vscode/blob/master/src/vs/workbench/parts/quickopen/browser/gotoLineHandler.ts#L98). Are you saying that this API talks about characters and not columns?

@bpasero bpasero added the info-needed Issue requires more information from poster label Oct 27, 2016
@bpasero bpasero added this to the Backlog milestone Oct 27, 2016
@alexdima
Copy link
Member Author

alexdima commented Oct 27, 2016

@bpasero I haven't seen a screenshot, but apparently there is somewhere some UI string the widget shows that mentions "column". That UI string should mention "character" instead of "column".

@mgerner Can you please include a screenshot.

@bpasero
Copy link
Member

bpasero commented Oct 27, 2016

@alexandrudima it is this one:

screen shot 2016-10-27 at 17 29 54

So when I pass a startColumn to the editor it will actually put it at the character position I give it?

@alexdima
Copy link
Member Author

@bpasero Please change your nls string such that it shows:
"Go to line 45 and character 34"

@bpasero bpasero modified the milestones: October 2016, Backlog Oct 27, 2016
bpasero added a commit that referenced this issue Oct 27, 2016
@bpasero bpasero assigned alexdima and unassigned bpasero Oct 27, 2016
@bpasero
Copy link
Member

bpasero commented Oct 27, 2016

screen shot 2016-10-27 at 17 55 57

I fixed the one in goto line quick open but I still see more places where we talk about column when it should maybe be character?

Bouncing back to you to decide if those are valid. I think they are in markers land mainly.

@alexdima
Copy link
Member Author

alexdima commented Oct 27, 2016

Thank you. The editor configurations talk about the correct columns. i.e. a ruler at column 4 will be rendered possibly after character 1 if that character is a tab (and tabs are configured to be equal 4). Same is for wrappingColumn.

giving to @dbaeumer and @sandy081 to think about their use-cases.

@alexdima alexdima assigned dbaeumer and sandy081 and unassigned alexdima Oct 27, 2016
@dbaeumer dbaeumer modified the milestones: November 2016, Backlog Oct 28, 2016
@dbaeumer
Copy link
Member

Makes sense to me to use character consistently.

@sandy081
Copy link
Member

To me too.

sandy081 added a commit that referenced this issue Oct 28, 2016
@sandy081 sandy081 removed their assignment Oct 28, 2016
@sandy081
Copy link
Member

Changed it for markers panel.

@dbaeumer
Copy link
Member

Fixed except for the cases where the doc was referring to properties. Since these properties are API and I didn't want to break the API I left them with column.

@chrmarti chrmarti added the verified Verification succeeded label Dec 9, 2016
@chrmarti
Copy link
Contributor

chrmarti commented Dec 9, 2016

gotoLine.ts has 3 occurrences of 'column'. argv.ts has 3 related to 'go to line'. Reopening.

@chrmarti chrmarti reopened this Dec 9, 2016
@chrmarti chrmarti added verification-found Issue verification failed and removed verified Verification succeeded labels Dec 9, 2016
@dbaeumer dbaeumer assigned bpasero and unassigned dbaeumer Dec 12, 2016
@dbaeumer
Copy link
Member

As discussed assigning to you.

@bpasero bpasero removed info-needed Issue requires more information from poster verification-found Issue verification failed labels Dec 12, 2016
@bpasero bpasero modified the milestones: January 2017, November 2016 Dec 12, 2016
@alexdima alexdima added the verified Verification succeeded label Jan 27, 2017
@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 18, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

5 participants