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

Change to references' lineText property breaks find-all-references in emacs #25489

Closed
sandersn opened this issue Jul 6, 2018 · 1 comment
Closed
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue

Comments

@sandersn
Copy link
Member

sandersn commented Jul 6, 2018

var g = function (x) {
  console.log(x)
  return x/**/
}

Find all references at x (any of them) in emacs.

Expected behavior:
Three lines:

test.js
  6:var g = function (x) {
  7:  console.log(x)
  8:  return x

Actual behavior:
Crash with emacs' inscrutable equivalent of index out of bounds ("let: Args out of range: 18, 19"). Deleting the body of tide-annotate-line stops the crash and exposes the real problem:

test.js
  6:x
  7:x
  8:x

lineText was previously the entire line text and is now just the text of the identifier. I am pretty sure that #25283 caused this change. I think it is unintentional and incorrect based on the description in protocol.ts:

        /** Text of line containing the reference.  Including this
         *  with the response avoids latency of editor loading files
         * to show text of reference line (the server already has
         * loaded the referencing files).
         */
        lineText: string;

Actually setting up emacs + tide + custom tsserver + custom logger is cumbersome and probably not necessary. Here is the before/after json object for this request:

2.9.2

{"seq":0,"type":"response","command":"references","request_seq":"200","success":true,"body":{"refs":[
    {"file":"/home/n/src/test/test.js","start":{"line":6,"offset":19},"lineText":"var g = function (x) {","end":{"line":6,"offset":20},"isWriteAccess":true,"isDefinition":true},
    {"file":"/home/n/src/test/test.js","start":{"line":7,"offset":17},"lineText":"    console.log(x);","end":{"line":7,"offset":18},"isWriteAccess":false,"isDefinition":false},
    {"file":"/home/n/src/test/test.js","start":{"line":8,"offset":12},"lineText":"    return x }","end":{"line":8,"offset":13},"isWriteAccess":false,"isDefinition":false}],"symbolName":"x","symbolStartOffset":12,"symbolDisplayString":"(parameter) x: HTMLElement"}}

3.0

{"seq":0,"type":"response","command":"references","request_seq":"203","success":true,"body":{"refs":[
    {"file":"/home/n/src/test/test.js","start":{"line":6,"offset":19},"end":{"line":6,"offset":20},"lineText":"x","isWriteAccess":true,"isDefinition":true},
    {"file":"/home/n/src/test/test.js","start":{"line":7,"offset":17},"end":{"line":7,"offset":18},"lineText":"x","isWriteAccess":false,"isDefinition":false},
    {"file":"/home/n/src/test/test.js","start":{"line":8,"offset":12},"end":{"line":8,"offset":13},"lineText":"x","isWriteAccess":false,"isDefinition":false}],"symbolName":"x","symbolStartOffset":12,"symbolDisplayString":"(parameter) x: HTMLElement"}}
@sandersn
Copy link
Member Author

sandersn commented Jul 6, 2018

@ananthakumaran you should be aware of this bug. Emacs breaks but VS and VS Code don't seem to be affected. VS Code uses the same references command so I am not sure why.

@mhegazy mhegazy added Bug A bug in TypeScript Fixed A PR has been merged for this issue labels Jul 6, 2018
@mhegazy mhegazy added this to the TypeScript 3.0.1 milestone Jul 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue
Projects
None yet
Development

No branches or pull requests

2 participants