Skip to content

Conversation

Lain62
Copy link
Contributor

@Lain62 Lain62 commented Feb 21, 2025

Added a simple odin lang support https://odin-lang.org/

it seems to be working well.

Lain62 added 2 commits March 2, 2025 05:35
- Types and Variables are declared basically the same anyway
  so combining them seems like a good idea
@diniamo
Copy link

diniamo commented Mar 3, 2025

@Lain62 what will it be then? \\s*\\bJJJ\\s*: should be used if we want to be 100% correct. However a trade-off could be made, because realistically you wouldn't search for functions and types that aren't declared in the name :: (proc) format, meaning we could go back to something like what you did originally, greatly reducing the number of results. Not sure if the second option is allowed in this project, but if it is, and you decide to do that, make sure to add a comment in the code, explaining this.

@Lain62
Copy link
Contributor Author

Lain62 commented Mar 4, 2025

I think its more important to be 100% correct, and to have the biggest coverage. Don't really want a situation where people get frustrated for it not finding something even if its rare. So I think keeping it as it is, is the best.

@diniamo
Copy link

diniamo commented Mar 4, 2025

But it's not 100% right now, since functions can also be declared with =.

@diniamo
Copy link

diniamo commented Mar 4, 2025

Oh and one more thing, I would change the generic regex to include the optional type specifier and the second :/=. Might avoid some more cases.

@jacktasia
Copy link
Owner

Thanks @Lain62 and @diniamo! All the tests pass, and this looks like a safe change, so I am ready to merge whenever you both are happy, assuming those two things stay true. So, please let me know; also, If you want to add "Odin" to the README.md language list, that would be cool, or I can do it post-merge. Thanks again

@Lain62
Copy link
Contributor Author

Lain62 commented Mar 4, 2025

alright I've added = to functions (forgot this exists) and added the optional type specifier and the second :/= for the generic regex. let me know if there's anything that needs changing.
also added odin to readme

@diniamo
Copy link

diniamo commented Mar 5, 2025

Functions can also have an identifier, e.g., var: proc() : proc() {} and var: proc() = proc() {} are valid. I'm not sure I see the point of separating functions and variables.

@Lain62
Copy link
Contributor Author

Lain62 commented Mar 5, 2025

okay ngl, i did not know you can put identifiers on functions.
removed it now

Copy link

@diniamo diniamo left a comment

Choose a reason for hiding this comment

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

Alright, I think that should work. I don't exactly understand the part of the regex that matches the type identifier, but it seems to have resolved every obscure test case I could throw at it, so we should be good.

I will make a note of this, but I'm fairly certain that we won't be able to handle very obscure cases like this:

var : struct {
    a: i32
} : { a = 1 } fmt.println(var)

is valid Odin. However as I said, I don't think we can account for that.

@Lain62
Copy link
Contributor Author

Lain62 commented Mar 5, 2025

yeah i don't think its possible to account for it. i think it should be good now

@diniamo
Copy link

diniamo commented Mar 5, 2025

@jacktasia Looks like we are ready to go.

Copy link
Owner

@jacktasia jacktasia left a comment

Choose a reason for hiding this comment

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

Thanks!

@jacktasia jacktasia merged commit f657254 into jacktasia:master Mar 5, 2025
6 checks passed
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.

3 participants