Skip to content

Conversation

Axlefublr
Copy link
Contributor

notably, I didn't lose the jq injection while bringing in the new queries
also adds an injections for fish -c

@Axlefublr
Copy link
Contributor Author

oh whoops, my personal change got in there
but I'll keep it unless asked to do otherwise: the default highlight query highlights the shebang as a directive; I've never seen it be highlighted that was so made it highlight as a comment again
I feel most users will be expecting a shebang to look like a comment, not as a directive

@Axlefublr
Copy link
Contributor Author

my bad, turns out helix can comprehend @number, it's just my theme that didn't set a color for it. the shebang directive -> comment change is still there though

@yerlaser
Copy link
Contributor

yerlaser commented Sep 5, 2025

Just tried applying your PR and noticed that highlighting broke on some items.
Capture
As you can see the comments on the first and third lines are highlighted differently than those on lines 5/6.
Also the $arg inside string isn't being highlighted.

@Axlefublr
Copy link
Contributor Author

Just tried applying your PR and noticed that highlighting broke on some items. Capture As you can see the comments on the first and third lines are highlighted differently than those on lines 5/6. Also the $arg inside string isn't being highlighted.

mhm, seeing the comment issue as well
($arg) not being green shows that it is highlighted, just maybe not how you expect; it's a theme thing. you can recolor identifier I think, to change how it's highlighted

@Axlefublr
Copy link
Contributor Author

pushed the comment fix; pretty neatly, it uses comment.documentation, so you could color them differently if you wish. just uses the color of comment by default though
thanks for noticing!

@Axlefublr
Copy link
Contributor Author

no clue what the error is in the ci btw. STATUS_STACK_BUFFER_OVERRUN is pretty vague 🧐
windows only, too, so I have no way to debug that

@RoloEdits
Copy link
Contributor

Perhaps the new parser is doing something that corrupts the stack? As this PR itself adds no unsafe code, it seems like it must be the C parser that has issues. Unless there are flags for only windows and only linux behavior in the parser, there might also be problems on linux as well, just not triggering a runtime exception. Some kind of UB.

@Axlefublr
Copy link
Contributor Author

fascinating! thank you for the inspection

@Axlefublr
Copy link
Contributor Author

I'm gonna try to run the ci through again, after removing all queries; to test if queries are part of the issue or not

@Axlefublr Axlefublr force-pushed the Axlefublr/nushell_parser branch from 3035114 to 1ab2835 Compare September 7, 2025 10:40
@Axlefublr
Copy link
Contributor Author

with no queries at all, the tests did pass; so the issue is likely with one of the queries. I'm going to bring back each file one by one until I find the offending one

@Axlefublr
Copy link
Contributor Author

it's not the folds

@Axlefublr
Copy link
Contributor Author

not the highlights

@Axlefublr
Copy link
Contributor Author

macos now?? seemed like a networking error though, I'll try again

@Axlefublr
Copy link
Contributor Author

not the indents

@Axlefublr
Copy link
Contributor Author

not the textobjects

@Axlefublr
Copy link
Contributor Author

not the uhhh.... anything? must've been the wind?
I literally changed nothing, and now it works 😌
ready for merge! lmao

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we have a lot of missing captures... do I just like, delete them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

delete the queries for captures we don't have in helix (and can't be represented as a different capture), I mean

Copy link
Member

Choose a reason for hiding this comment

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

Yeah I think the neovim queries cover a lot more than the textobjects we have. We can drop or comment-out the ones that don't fit into the Helix queries

Copy link
Contributor Author

Choose a reason for hiding this comment

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

mhm, that's what I did. good to know

@iniw
Copy link

iniw commented Sep 16, 2025

Do you mind testing this PR out on this file? https://github.com/iniw/system/blob/master/modules/shell/config/config.nu

Currently helix produces the wrong highlighting for everything past line 66, which contains an @example attribute with inline nushell code.

Also, inline nushell code in @example attributes are currently poorly highlighted in general.

@Axlefublr
Copy link
Contributor Author

dear github,
please add the sob emoji react for this exact usecase.
thank you.

(i.e. I'll look into it but ffs 😭)

@yerlaser
Copy link
Contributor

dear github, please add the sob emoji react for this exact usecase. thank you.

(i.e. I'll look into it but ffs 😭)

I experienced broken syntax highlighting in Nushell code all the time, so no need to be frustrated.
I think this PR already makes it better in many ways, I am especially happy about automatic indenting.

@Axlefublr
Copy link
Contributor Author

for context, I'm not part of the nushell team, nor is the treesitter parser / queries mine; I just took them from the upstream repo to sync helix up to speed, with some minor changes. so that's where my “sir, I just work here” 😭ation comes from hahaha

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.

5 participants