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

Added 'show context' support for Zig #729

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

chrboesch
Copy link
Contributor

closes #728
I did already in v2: #564

zig = {
Block = true,
InitList = true,
FnCallArguments = true,
Copy link
Owner

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,
Copy link
Owner

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,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same for this

@chrboesch
Copy link
Contributor Author

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

@lukas-reineke
Copy link
Owner

Have you read :help ibl.config.scope? Scope has semantic meaning.

@chrboesch
Copy link
Contributor Author

Have you read :help ibl.config.scope?

Yes, I did.

Scope has semantic meaning.

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.
Here an example from standard library 'io.zig', line 109 ff.:

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:
pub fn GenericReader( -> (Block)
comptime Context: type, -> (ParamDeclList)
/// If the number of bytes read is 0, it means end of stream. -> (doc_comment)
return struct {" -> (Block)
context: Context, -> (ContainerDecl)

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. 😄

@chrboesch
Copy link
Contributor Author

Almost forgot, I really like your plugin and am glad it exists and works so wonderfully (even with Zig). 👍

@lukas-reineke
Copy link
Owner

because there is already a scope inside the parameter declaration

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.

config.scope *ibl.config.scope*
Configures the scope
The scope is *not* the current indentation level! Instead, it is the
indentation level where variables or functions are accessible. This depends
on the language you are writing.
Example: ~
In Python, an `if` block is not a new scope, variables defined inside `if`
are accessible outside. The scope is the function `foo`.
(please don't actually write code like this)
>python
def foo();
┋ if True:
┋ a = "foo █ar"
┋ # ↳ cursor here
┋ print(a)
<
In Rust on the other hand, `if` blocks are a new scope. Variables defined
inside are not accesible outside. Indent-blanklines scope reflects this, and
the scope is just the `if` block.
(this code would not compile)
>rust
fn foo() {
if true {
┋ let a = "foo █ar";
┋ // ↳ cursor here
}
print(a);
}
<

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?
Structs are also not a scope.

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. [...]
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.

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.

@chrboesch
Copy link
Contributor Author

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:

265403827-a9d2426f-56a4-44bd-8bb5-2a3c5f5ca384

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.

@lukas-reineke
Copy link
Owner

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 if blocks in Python, because in Python they are not a scope! While in other languages like Rust and Lua if blocks are.
Not every new indent is a scope, a scope is defined by the language.

Maybe this blogpost helps?

@chrboesch
Copy link
Contributor Author

I'm not really sure how else I can explain it other than what I wrote in the documentation.

But is the behavior shown in the screenshot the correct one, i.e. the desired one?

@lukas-reineke
Copy link
Owner

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 if block is a new scope.

@chrboesch
Copy link
Contributor Author

Now the screenshots from Zig. Do you think they show the right behavior?

bl1
bl2
bl3
bl4
bl5
bl6

@lukas-reineke
Copy link
Owner

The first 2 yes (probably, as I don't know zig), the others no.
Function arguments and structs are not a scope.

@chrboesch
Copy link
Contributor Author

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.

@lukas-reineke
Copy link
Owner

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.

@chrboesch
Copy link
Contributor Author

We are working on current indent highlight in #649, which is probably what you want?

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.

@lukas-reineke
Copy link
Owner

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.

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.

v3 Zig does not show context
2 participants