Skip to content

Multiple bugfixes#47

Closed
FalsePattern wants to merge 37 commits intormordechay:mainfrom
FalsePattern:main
Closed

Multiple bugfixes#47
FalsePattern wants to merge 37 commits intormordechay:mainfrom
FalsePattern:main

Conversation

@FalsePattern
Copy link
Copy Markdown
Contributor

@FalsePattern FalsePattern commented Jan 14, 2025

Superseded by:
#51
#52
#53
#54

@rmordechay
Copy link
Copy Markdown
Owner

@FalsePattern great work!
Unfortunately I won't have time this weekend nor next week. I will try to find time next weekend.
I had a quick look at the PR and it seems like you did much needed changes. Thx.

@FalsePattern
Copy link
Copy Markdown
Contributor Author

Added 2 more minor highlighting fixes, the text attribute change should make light theme automatic default colors a bit nicer

@FalsePattern
Copy link
Copy Markdown
Contributor Author

FalsePattern commented Feb 5, 2025

I improved the "shaderpack absolute path" hack a little bit, now there's a way to also use it in non-Minecraft projects via a marker file (.glsl_idea_root) in the "root folder" of the shader code. I wanted to do it via source sets, but that just adds way too much IDE jank to a simple problem.

I added a section to the end of the readme so that users know about it:
image

@rmordechay
Copy link
Copy Markdown
Owner

@FalsePattern
I didn't forget you PR. I have no time to look into this. Unfortunately I think I will have time only next week or so.

@rmordechay
Copy link
Copy Markdown
Owner

I don't mind changing stuff but I guess we should discuss that before changing. There are some things that I chose with purpose, and also, it made the PR hard to review as it became pretty big. So there are some things that I would like to have as they were for this PR, and if you want/need some changes then please make a separate PR:

  1. Please revert Gradle and in general all the build changes. If there are some good reasons to change them, let's do it in a different PR.
  2. Please move grammar dir to its original location. I know it doesn't resolve in the grammar but I think such important files should be in the root path. It helps for navigation.
  3. Revert to the original GeneratedParserUtil.java. It has logic that the original file doesn't have.

Comment thread README.md
```
Assuming you're developing with Intellij (and you want to develop with Intellij):
1. **Generate grammar**. Execute the `generateGrammarClean` task from _gradle.build_ file or under _Tasks/other_ if you use the Gradle tab.
1. **Generate grammar**. Execute the `generateGrammars` task from _gradle.build_ file or under _Tasks/grammarkit_ if you use the Gradle tab.
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

What's the reason to change the name?

Comment thread CHANGELOG.md
@@ -1,4 +1,28 @@
# GLSL Plugin Changelog
## [Unreleased]
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

You could already put the next version here, 1.1.5

}

/**
/**2
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

i assume the '2' is a mistake

Comment thread gradle.properties
@@ -1,2 +1,7 @@
pluginVersion=1.1.4
pluginVersion=1.1.5-dev
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

There should be no dev versions. Just put 1.1.5 for the next version.

private fun getPostfixSelectionType(`postfixExpr`: GlslPostfixFieldSelection): GlslNamedType? {
if (postfixExpr.postfixStructMemberList.isEmpty()) return null
val lastExpr = postfixExpr.postfixStructMemberList.last()
if (lastExpr.variableIdentifier == null)
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

All if's in the project have parenthesis

@FalsePattern FalsePattern marked this pull request as draft March 4, 2025 22:59
@FalsePattern
Copy link
Copy Markdown
Contributor Author

I'll split this PR into smaller ones and undo the buildscript changes either this weekend or a few weeks after, i don't have a lot of free time at the moment

@FalsePattern
Copy link
Copy Markdown
Contributor Author

PR split map:
image

Legend:
Blue - #51
Green - #52
Red - #53
Magenta - #54
White - dropped

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