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

More fixes for POC rename symbol #2

Open
wants to merge 127 commits into
base: poc_rename_symbol
Choose a base branch
from

Conversation

timgent
Copy link

@timgent timgent commented Nov 10, 2022

Few more fixes on this:

  • Other language servers I've used were just returning nil for the version, which seems to have fixed the [coc.nvim] Error on applyEdits: Error: file:///.... changed before apply edit issue I was having before
  • Find references when on a variable definition also finds the definition itself, so we were getting duplicate entries in the list of edits because of that (causing renames to fail due to overlapping ranges). Just done a Enum.uniq on the list of changes to fix that
  • For files other than the one you are on it now returns the URI in the correct format (it wasn't working before)

Stuff I think still needs doing:

  • There's a small bug with ElixirSense where it doesn't find references for a variable if the definition is on the line after the variable name, something like:
cow = 
  "moo"

I've now raised a PR for this on the elixir sense repo - elixir-lsp/elixir_sense#172 - it has been merged now

  • - Tests for rename prepare functions
  • - Preparing to rename currently fails for a function defined in another file, so that needs fixing

Things for follow-up PRs:

  • - Rename module functionality
  • - Find references (and hence rename) doesn't work when a usage of a variable is in a list comprehension (this is more of an issue with existing Elixir Sense functionality rather than the rename functionality, but it would be nice to fix)

Btw do you have any guidance on testing with VS code? I mostly use Vim, but would like to test things out on VS code too. UPDATE: This seems helpful - https://dragoshmocrii.com/fix-vscode-elixirls-intellisense-for-code-imported-with-use/

gofenix and others added 30 commits February 11, 2022 12:11
* elixir-lsp#673 fix hover bug for

* use function_exported to check module be loaded

Change-Id: I8e3c3cde067a7d3313422c0a94f4c4a464bd37de

* remove rescue and use if instead of nested cond

Change-Id: Ia85056971d7333df5d6a7cf6d3a6bd21c87635eb

Co-authored-by: zhuzhenfeng.code <zhuzhenfeng.code@bytedance.com>
compatibility improvement: only return variable type and pagination properties when supported by client
run evaluate in a stacktrace frame if specified
…r-lsp#675)

* add support for terminateThreads request

* add support for pause

* format
…nto elixir_sense (elixir-lsp#677)

* add utility functions for utf8-utf16 conversions

* fix numerous cases of incorrect utf16 positions
…lixir-lsp#678)

* remove not supported SetExceptionBreakpoints request handler

Clients should only call this request if the capability ‘exceptionBreakpointFilters’ returns one or more filters.
No way to implement it via :int module

* implement function breakpoints

* test erlang breakpoints

* fix small memory leak when unsetting last breakpoint in file

* add more breakpoint tests

* make tests more synchronous

* add function breakpoints tests

* interpret modules

* extract and test mfa parsing

* run formatter

* Update readme

* cleanup

* extract and test erlang binding processing

fix some isses when var is usend more than 10 times
add explicite ordering by variable instance
discard underscored variables

* breakpoint conditions support added

do not warn when setting already set breakpoint

* update readme

* log when expression crashes

* add support for conditional function breakpoints

* update readme

* add support for breakpoint hit condition

* readme updated

* implement log message

* readme updated

* add support for terminateThreads request

* add support for pause

* make continue, next, stepIn, stepout requests conform to DAP 1.51

Fixes elixir-lsp#669
reworks fixes for elixir-lsp#455

* cleanup :int workaround

* tests wip

* format
* add support for hover and clipboard eval in debugger

* use empty string
* fix typo

* warn about not supported client options

* Add support for debugger completions
…sp#681)

* delete duplicated capability

* add process messages as debugger variable scope

* improve display of keyword variables

* add process info as debugger variable scope
…ir-lsp#684)

* Suggest an appropriate module name when auto-completing the 'defmodule' snippet

* use processed file_path instead of raw file:// uri for determining appropriate module_names

* add umbrella_app test to guard against future regressions

* special case common Phoenix folders when suggesting module names

* fix broken test

Some plugin or setting in my editor trimmed extra spaces from the lines
but in this case it cause a test to break by changing the cursor position
of the auto-completion trigger
* Consistent headers (getting-started section)
* Fix wrong header for vscode getting-started page
* Removes trailing spaces
* Fixes typos
do not return negative numbers as those are forbidden by the LSP
properly treat 0 as unknown position
add missing handling for positions with column
add missing handling for positions as ranges
if file the diagnostics refer to is not open fall back to loading it from the file system
do not return lines and columns if we cannot load the file

Fixes elixir-lsp#695
* check if loading apps is still needed

* Revert "check if loading apps is still needed"

This reverts commit d03cfe5.

* rename function to match what it actually do

* extract diagnostic related code

* extract common mix.exs code

* fix tests
lukaszsamson and others added 20 commits October 31, 2022 07:49
Path.wildcard is not working on umbrella level
Right now, this change is a no-op, but if and when this change
elixir-lsp/elixir_sense#165
lands, this will quiet elixir_sense's logging in the prod
environment, which will prevent logspam in lsp buffers when
elixir_sense encounters invalid code.
position should trim to the end of the line
This triggers race conditions in elixir compiler
Fixes elixir-lsp#763
lukaszsamson and others added 4 commits November 12, 2022 16:37
…ixir-lsp#768)

- so, this behavior wasn't working for me in vs-code, so I hopped into
    the repo to write a failing test, but the test passed right away (🤔)
    and I'm now seeing the behavior _sometimes_ work and sometimes not
    in vs-code
  - conclusion is that the issue is likely in the vs-code extension, but
    I figured I'd submit a PR with the regression test if it's desired
@timgent
Copy link
Author

timgent commented Nov 13, 2022

@Tuxified I think this is pretty much ready, though recent changes to the elixir sense dependency seem to have broken things :( I need to take a deeper look into that but let me know if you have any suggestions. Adding the trace argument seems to fix most things, but the test for renaming a function defined in another file is now broken :(

@timgent
Copy link
Author

timgent commented Nov 13, 2022

Added some test setup bits and it's all working again now 👍

Let me know if you're happy to merge this to get your PR updated, or if it would be easier for me to raise a new PR with the main repo and then I can follow up any comments too. I'm happy either way :)

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