-
-
Notifications
You must be signed in to change notification settings - Fork 107
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
Added 'show context' support for Zig #729
Conversation
zig = { | ||
Block = true, | ||
InitList = true, | ||
FnCallArguments = true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know zig, but I'm pretty sure function arguments are not a new scope
ContainerDecl = true, | ||
SwitchExpr = true, | ||
IfExpr = true, | ||
ParamDeclList = true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is probably also not a separat scope
InitList = true, | ||
FnCallArguments = true, | ||
IfStatement = true, | ||
ContainerDecl = true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same for this
What shall I say, I figured everything out with 'Treesitter inspect" and it works very well. Since I made these changes on my local machine, I have the same scope as in v2 and how every Zig developer needs it. You can try it out for yourself, just open a Zig source code, for example this one: zig calc |
Have you read |
Yes, I did.
That's right. But Zig has a strict formatting requirement. For example, if you include in 'Structs' only the 'Block' and not the 'ParamDeclList' in the list, then the scope will only work from the function block. But this is wrong, because there is already a scope inside the parameter declaration. pub fn GenericReader(
comptime Context: type,
comptime ReadError: type,
/// Returns the number of bytes read. It may be less than buffer.len.
/// If the number of bytes read is 0, it means end of stream.
/// End of stream is not an error condition.
comptime readFn: fn (context: Context, buffer: []u8) ReadError!usize,
) type {
return struct {
context: Context, The cursor line by line: It took me some time to understand what is needed in the list so that the scope is not lost when browsing through the source code. The result is my pull request. So no matter what it looks like, I've been working with it for a few days and it works as intended. So my problem is solved. What you do with it is up to you. I just find it very exciting that you are discussing this with me instead of a) adopting it and/or b) convincing yourself that it is right. You say you don't know anything about Zig. You don't have to. It is enough to open any source code, preferably from the standard library, and scroll through it. Feel free to exclude parts from my list (of which you say it is not scope) and see what happens. I think at the latest then you will take my list as it is, because it works. And until now I thought that was what mattered. If I am wrong, please give me more information than 'have you read this and that'. Because as I have noted in another place, the help at this point is unfortunately not so meaningful. 😄 |
Almost forgot, I really like your plugin and am glad it exists and works so wonderfully (even with Zig). 👍 |
I still don't think you understand the semantic meaning of what scope is. This is why I asked you to read the documentation, I explained it there, with examples. indent-blankline.nvim/doc/indent_blankline.txt Lines 365 to 397 in 9301e43
You can also read the Wikipedia definition. By the very definition of what parameter declarations are, they cannot be an isolated scope. Otherwise, how would functions work if you can't access parameters from the function body?
What's up with this attitude? Your PR is wrong, if you continue to get this defensive and arrogant when I point it out, I will just close the PR and not engage with you anymore. |
Please explain to me what I should do. So what do you want the result to be? How should the program behave? I can't do anything with either the Python example or the Rust example. Both languages I am not familiar with. But I have now looked at a Python program in Treesitter and compared it to a Zig program. And then compared again with your screenshot: If this is the correct screenshot, Zig behaves exactly as shown, while Python does not. And with that, I'm confused to the max. So in order to understand what I should change, I must first understand what the result should be. For me as a programmer, the behavior as shown in the screenshot is the desired result. That's why I submitted my PR, because it makes Neovim behave exactly the same way with a Zig program. However, I take it from your comments that this behavior is not the goal at all. So we have a typical communication problem, one says x the other understands y. Let's try to solve this problem. |
I'm not really sure how else I can explain it other than what I wrote in the documentation. This plugin does not show the scope line on Maybe this blogpost helps? |
But is the behavior shown in the screenshot the correct one, i.e. the desired one? |
Yes, the screenshot is of lua code, and in lua an |
The first 2 yes (probably, as I don't know zig), the others no. |
I see. Then we really do have a different view of what we want to be displayed. For my screenshots I have now chosen small examples. In Zig, the lists and subnestings can become much larger, which is exactly why I need the display as I have built it. From the pure doctrine you are of course right, but from the praxix I need the display as shown in the screenshots. So if I now remove the parts you noted as out of scope, the plugin would be useless in combination with Zig. Maybe now you understand my dilemma. I'm now at loss. |
We are working on current indent highlight in #649, which is probably what you want? But that is not scope. Scope is defined by the language, it doesn't matter how large lists can become. |
Did not see yet, but will read, thanks. Last example to Zig, a 'return struct' (screenshot 4) corresponds to a class init function as for example in Java. So it is a function body like 'fn foo()' and for my understanding in scope. |
Maybe this is my lack of zig knowledge. In a language like rust a struct is definitely not a scope. But if it acts more like a function or class body it might be in zig. |
I am not sure what happened here. I like the plugin, and I love Zig, so I was a bit disappointed when the Zig is neither Python nor Rust. It also has compile-time code execution, which is often used. The tip to peek into Zig's standard library was good. The whole argument —it doesn't work like this in Rust, so it's probably wrong in Zig, too—is technically invalid to me. If I didn't know a language, I would never make such claims. Of course, there can be scope in Zig structs, and it's often the case. Micro-example: const S = struct {
field1: bool = true, // struct field
field2: []const u8, // struct field
const Self = @This(); // struct level type
var x: usize = 0; // struct "static variable"
// use struct-scoped Self type:
pub fn init() Self {
return .{
.field1 = x == 0, // use struct-scoped variable x
.field2 = "set",
};
}
}; See Container Level Variables. Moreover, every zig source file is (the equivalent of) a struct. So, in Zig, quite a lot is in scope. See Containers. Also, consider this fn foo(T: type) std.ArrayList(T) { return .{} }; Here, the type Given the above, a So IMHO, what @chrboesch did, is pretty accurate, as far as I can tell. |
Thanks @renerocksai, I created a fork with my changes and it has worked very well for me since then. |
Like I said above, this might very well be just my lack of zig knowledge. I'm still not against merging this, if I am confident enough that everything is correct. Unfortunately, I do not have the time to learn zig for this PR, so someone has to give me some convincing examples. Your example of the I am still not sure about the parameter list, though. |
My example was bad. My point was that a scope is opened in the parameter list. Whether So here is an easy example that shows it: pub fn startsWith(comptime T: type, haystack: []const T, needle: []const T) bool {
return if (needle.len > haystack.len) false else eql(T, haystack[0..needle.len], needle);
} Hope this helps. |
But
|
Why do you assume that you know better than actual Zig users? Comptime variables can be used in functions, e.g. to create other types, like in
Referring inside the parameter list (look at haystack and needle): Calling functions, passing around the parameter, inside of the parameter list: |
const std = @import("std");
// just a silly enumeration
const FloatOrInt = enum { MyInt, MyFloat };
// Return an instance of an ArrayList whose type is depending on **the value** of floatOrInt
pub fn NumberList(comptime floatOrInt: FloatOrInt, allocator: std.mem.Allocator) std.ArrayList(switch (floatOrInt) {
.MyInt => i32,
.MyFloat => f32,
}) {
return std.ArrayList(switch (floatOrInt) {
.MyInt => i32,
.MyFloat => f32,
}).init(allocator);
// note the .init() above. It doesn't return the type but an instance.
}
// here, floatOrInt is passed in and then used inside a switch expression inside the param list
pub fn contrivedIsEmpty(comptime floatOrInt: FloatOrInt, list: std.ArrayList(switch (floatOrInt) {
.MyInt => i32,
.MyFloat => f32,
})) bool {
return list.items.len == 0;
}
pub fn main() !void {
var gpa = std.heap.GeneralPurposeAllocator(.{}){};
defer _ = gpa.deinit();
const allocator = gpa.allocator();
const nl = NumberList(.MyFloat, allocator);
const is_empty = contrivedIsEmpty(.MyFloat, nl);
std.debug.print("{d}: {}\n", .{ nl.items.len, is_empty });
}
I think it's OK to speak of a scope here. |
That's exactly what I am saying,
It is extremely frustrating to deal with people that like you or OP who clearly do not understand what I am trying to tell you. And then in return try to lecture me. I think I was very patient with this PR so far. Have a good day |
closes #728
I did already in v2: #564