-
Notifications
You must be signed in to change notification settings - Fork 16
don't break inline param comment formatting #80
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
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| def fun1 [ | ||
| text: string # param comment | ||
| ] { $text } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| def fun1 [ | ||
| text: string # param comment | ||
| ] { $text } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Does this mean there won't be any change in the signature if there's a doc comment?
Just out of curiosity from eyeballing the code as I don't have a machine with me right now to test, I think custom completion expressions in signatures are missing?
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.
ya, it should stay the same if there are comments. I'm not sure if that's right or wrong.
This code is really just thrown together to try and get over the hump of no one working on it. I really needs like 1000 samples for the ground truth. The intent was to be able to create and input script and an expected script and have it run those without compilation. However, it's all hard coded right now. The more samples we can get with different syntaxes the better testing will be and the better formatting will be.
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.
Glad to see such great improvement in nufmt. It feels almost there that I can archive
topiary-nushelland let go, I thought it was an impossible task before a mature new-nu-parser.On the other hand, the signature thing is till annoying, and I'm afraid too much hack on nufmt doesn't worth the effort for issues that should be originally addressed on the parser side.
Having said that, would you like me to report new issues, like the mentioned "custom completion expression in the signature", or do you have any other plan of how we should proceed with this project?
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.
ya, please report issues. I may not fix them all. Some, like you say, are parser bugs. I definitely don't want you to archive topiary-nushell but I do want to prove (mostly to myself) that nufmt can actually work as a real-world formatter. I'm honestly not convinced right now. I still think i'll eat more code than it will format, but at least someone is trying to make it better.
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.
Right now, nufmt seems promising. And I think a single working and configurable formatter is enough for the community.
topiaryhas its own limit that won't make everybody happy. If we can overcome some more obstacles either by improving nu-parser/new-nu-parser or nufmt, then I thinktopiary-nushellhas fullfilled its task in history.