Skip to content
This repository has been archived by the owner on Jul 15, 2023. It is now read-only.

Rename fails when "---" is anywhere in the file #968

Closed
riosdavi opened this issue May 5, 2017 · 4 comments
Closed

Rename fails when "---" is anywhere in the file #968

riosdavi opened this issue May 5, 2017 · 4 comments

Comments

@riosdavi
Copy link

riosdavi commented May 5, 2017

Provided the following code:

01. func (cluster *Cluster) Update() {
02.	logger.Debug("--- Update begin")
03.	for _, device := range cluster.Devices {
04.
05.		objects, err := getObjectsByDeviceID(device.DeviceId)
06.		if err == nil {
07.			logger.Debug("--- Update count:", len(objects), " - deviceId:", device.DeviceId)
08.			err = updateObjectCount(objects)
09.		}
10.		if err != nil {
11.			logger.Error(err.Error())
12.		}
13.	}
14.	logger.Debug("--- Update end")
15. }

Renaming device to soDevice causes the following refactoring, where line4 (blank line) is replaced by a duplicate of line 5 with the old variable name (device), and lines 8, 9 and 10 are replaced by logger.Debug("Index:

01. func (cluster *Cluster) Update() {
02.	logger.Debug("--- Update begin")
03.	for _, soDevice := range cluster.Devices {
04.		objects, err := getObjectsByDeviceID(device.DeviceId)
05.		objects, err := getObjectsByDeviceID(soDevice.DeviceId)
06.			logger.Debug("--- Update count:", len(objects), " - deviceId:", device.DeviceId)
07.			logger.Debug("Index
08.			logger.Error(err.Error())
09.		}
10.	}
11.	logger.Debug("--- Update end")
12. }

Removing line 4 (empty line) causes the line 5 not being duplicated after the refactor but the result is almost the same.

Note: after removing the logging lines (logger.Debug and logger.Error), renaming worked as expected.

@ramya-rao-a
Copy link
Contributor

@riosdavi We havent changed anything in the rename area in quite a while.

The rename feature works by parsing the diff returned by the gorename tool.
The gorename tool uses the diff tool in your path.

Based on your description, looks like there is mistake with parsing.

Can you change the "--- Update" to just "-- Update" and try again?

My guess is that https://github.com/Microsoft/vscode-go/blob/master/src/diffUtils.ts#L175 is messing up your scenario.

@riosdavi
Copy link
Author

riosdavi commented May 8, 2017

@ramya-rao-a thanks for your quick reply. Effectively, replacing "---" by "--" solved the issue but I think a better algorithm should be used to detect diff output.
I would vote for such a fix.
Thanks!

@ramya-rao-a
Copy link
Contributor

@riosdavi Of course, a better logic is needed. I just asked you to to the replacing to confirm that https://github.com/Microsoft/vscode-go/blob/master/src/diffUtils.ts#L175 indeed is the root cause.

Nevertheless, that was put in place as a workaround for a bug in the diff module which has since been fixed, so I can remove that check altogether.

Thanks for reporting the issue and confirming my hypothesis.

Fixed with 1adae1f

The fix will be out in the next update which should be after about 2 or 3 weeks

@ramya-rao-a ramya-rao-a changed the title After upgrade to 0.6.61 version, variable renaming is not working Rename fails when "---" is anywhere in the file Jun 8, 2017
@ramya-rao-a ramya-rao-a added the bug label Jun 8, 2017
@ramya-rao-a
Copy link
Contributor

The fix is now available in the latest update (0.6.62)

@vscodebot vscodebot bot locked and limited conversation to collaborators Jan 24, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants