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

[display] nonsense field completion subject in nested switch #8194

Closed
Gama11 opened this issue Apr 21, 2019 · 3 comments
Closed

[display] nonsense field completion subject in nested switch #8194

Gama11 opened this issue Apr 21, 2019 · 3 comments
Labels
feature-ide IDE / Editor support test-needed

Comments

@Gama11
Copy link
Member

Gama11 commented Apr 21, 2019

class Main {
	static function main() {
		switch (nodeName) {
			case "p"
				switch (a) {}
		}
	}
}

When typing the : after the "p" here, Haxe returns field completion (?) with 0 items and this range as the subject:

"range": {
    "start": {
        "line": 3,
        "character": 3
    },
    "end": {
        "line": 4,
        "character": 17
    }
},

This then leads to some rather strange postfix completion:

When the inner switch is commented out, Haxe doesn't give a completion response at all (as expected, since completion was auto-triggered with : after a case).

Doesn't seem to be a regression (on the Haxe side at least). Original issue at vshaxe/vshaxe#337. Should be able to work around it by not allowing postfix completion if the completion was auto-triggered with a :.

@Simn
Copy link
Member

Simn commented May 21, 2019

Has nothing to do with nesting, the problem is this code in matcher.ml:

haxe/src/typing/matcher.ml

Lines 574 to 578 in 1ce52fa

if ctx.is_display_file && DisplayPosition.display_position#enclosed_in p then begin match eo,eo_ast with
| Some e,Some e_ast -> ignore(TyperDisplay.display_expr ctx e_ast e DKMarked with_type p)
| None,None -> ignore(TyperDisplay.display_expr ctx (EBlock [],p) (mk (TBlock []) ctx.t.tvoid p) DKMarked with_type p)
| _ -> assert false
end;

This came from fixing #5656. I cannot reproduce that anymore after removing these lines though, so I guess I'll just optimistically do that.

@Simn
Copy link
Member

Simn commented May 21, 2019

Not sure how to test this because it depends on wasAutoTriggered: true.

@Simn Simn added test-needed and removed bug labels May 21, 2019
@Simn
Copy link
Member

Simn commented May 21, 2019

Simplest test case is this:

class Main {
	static function main() {
		switch ("p") {
			case "p":
				"foo";
		}
	}
}

This gave string completion from the "foo" before my fix.

@Simn Simn added this to the Backlog milestone May 22, 2019
@Simn Simn removed their assignment Jul 1, 2019
@Simn Simn closed this as completed in bf9d781 Aug 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-ide IDE / Editor support test-needed
Projects
None yet
Development

No branches or pull requests

2 participants