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 two options: set 1 line above functions, ignore '/' symbols in $node references #3

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

Conversation

tobydjones
Copy link

I have added two options:

  1. An option to add 1 empty line above functions instead of 2
    This defaults to 2 lines, as in the original addon

  2. An option to ignore '/' symbols in $node references when adding spaces around operators.
    Note: I have added this option by treating $node references like text in quotes.
    To delimit $node references I have assumed they start with $ and end with either either . = : tab space or EOL
    As I am not sure if this is a complete list of delimiters, or if this option might somehow cause problems, I have labelled it as experimental, and turned it off by default.

@didier-v
Copy link
Owner

didier-v commented Oct 3, 2024

Hello ! I didn't expect any issues or pull requests on this little project. Sorry for the late reply.
Thank you. Your suggestions are interesting.

I have a few comments before merging.

  • About characters that delimit a node reference, a node could be a parameter of a function like this : func($Node/subNode). It could also be in an array or a dictionary [$NodeA/NodeB,$NodeA/NodeC]or {'key':$NodeA/NodeB}. Maybe there are other cases, I'm not sure. Anyway, if you use a regex instead of a list of if-else, you won't have to worry so much about the delimiters. Maybe try something like this regex ?
var regex = RegEx.new()
regex.compile(r"\$(\w+)(\/\w+)*")
...
  • In the options, check boxes are not good UI for exclusive options, even if you program them to be exclusive. I suggest to replace them with a OptionButton with 3 values 0,1, or 2 lines. You could also change their style to make them look like radio buttons. It's up to you.
  • Please don't alter the existing lines in the test.gd file, only add lines for new tests. This includes the empty lines or lines containing tabs or spaces only.

Also, could you squash your commits into one ?

@didier-v
Copy link
Owner

didier-v commented Oct 4, 2024

I made a few more tests with node names. Godot accepts any unicode characters, including for example emojis. The only forbidden characters are . : @ / " %
However, if a identifier contains anything that is not a latin letter, number, or underscore, the syntax must be with simple or double quotes, like $"Node+with=other-char acters" or $'Node+with=other-char acters'.
So we need to test both versions. For the first one, the regex would be \$[A-Za-z0-9_]+(\/[A-Za-z0-9_]+)*. For the second one, it might be more difficult to test it with a regex.

This example is the worst: the node name No'de is correct, but you must use it with this syntax only $"No'de" because the editor obviously can't handle $'No'de'.

@wyattbiker
Copy link

The Editor catches errors so you don't have to account for them in your aforementioned cases. E.g. $No'de will give an error anyway.

In addition to $node names, there also the %nodes that are referenced using % (enabled by Access as Unique Name in the IDE). Example %ViewMessages.disabled = true

You can prevent the GDBeautify from running if there is a syntax error in the code. This function below called from _on_beautify_pressed() checks for errors in the script.

func check_syntax() -> int:
	var error = current_script.reload()
	if error != OK:
		# If it has errors, we printerr to Godot and end this function right here
		printerr("GDBeautify: Syntax Error %s in Script. Cannot run. " % [error])
		return error
	# If there was no errors, we print a "succesful!" message or something like that
	print("GDBeautify: Script loaded with no errors!")	
	return OK

## Beautifies the current script.
func _on_beautify_pressed():
	if check_syntax():
		return
###.....
###....

@tobydjones
Copy link
Author

Thanks for looking at my changes :)
I'll see if I can change the node delimiter to a regex
I will change the check boxes to OptionButtons
I didn't intend to alter the existing lines in test.gd, sorry about that, I'll undo that.
I don't know how to squash my commits into one, but I'll try.

@didier-v
Copy link
Owner

@wyattbiker If you keep working on that feature, make sure to rebase to master. I have moved and renamed some files.

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