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

Syntax inconsistency lint: throw warning at both sites of inconsistency #174

Merged
merged 3 commits into from
Dec 29, 2023

Conversation

FPtje
Copy link
Owner

@FPtje FPtje commented Dec 29, 2023

The syntax inconsistency warning could be a little bit annoying to fix. You would get a warning at some point notifying of inconsistent syntax, but it wouldn't actually show you where the other side of the conflict is.

This was mentioned in #170 (comment) by @robotboy655

Here's how things look now:
Screenshot_20231229_185922

The pattern you see is that the inconsistency warning always comes in pairs. For the first two functions it warns, but for the third it doesn't. For the fourth and fifth it does again. The trick is that for each instance of inconsistency, it only warns once. The result: if the top half of your file uses one format, and the other half uses the other, then you'll nicely get two warnings in the middle!

@FPtje FPtje merged commit 5ea82cf into master Dec 29, 2023
2 checks passed
@FPtje FPtje deleted the fp/syntax-consistency-show-original branch December 29, 2023 18:25
@robotboy655
Copy link

I see what you did here and I think this is useful, but what I was actually referring to is also including the place previous instance of the inconsistency in the warning message itself, like shadowed variables do, like so: Inconsistent use of '&&' and 'and'. See pervious usage at Line 37 Column 7 or something to that effect.

This enables us to have a neat button in an IDE (in my testing case - VSCode) to go the other line:
image


On an unrelated to this PR note, this might be wishful thinking on my part, but I have been thinking of a possibility of a "live" mode for the glualint.exe, i.e. feeding in code to stdin would immediately spit out warnings, allowing reusing the process without having to create a new one for each file. This should greatly speed up the VSCode extension for large projects.

Alternatively, perhaps JavaScript/TypeScript builds could be released in some capacity (or at least instructions for that) so that the separate .exe is not even necessary for the extension.

I am not sure what the best approach is for this, but the main issue is just creating dozens of processes or more in short timespan does severely affect performance on Windows, making the entire system unusable in my case.

@FPtje
Copy link
Owner Author

FPtje commented Dec 29, 2023

This enables us to have a neat button in an IDE (in my testing case - VSCode) to go the other line:

I see your point, but I prefer this solution, because it doesn't depend on the editor parsing the error, and there is still a neat button: next/previous error. With the property that pairs of inconsistency warnings are close, this should get you to the other location in one or two steps.


On the other note: creating many processes on Windows affects performance that much? That's annoying 🤔
I wonder how a live mode should work. It cannot just take a stream of stdin and stream out the errors, because it would not know when a piece of code is parseable, and it would confuse the variables of different files. Maybe with \0 separators it should be possible.

As for a JavaScript build, that should be technically possible (because glualint-web exists), but performance is going to be way, way worse than the executable.

Could you write an issue?

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

Successfully merging this pull request may close these issues.

2 participants