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

vim: update configuration to include theme files #4893

Merged
merged 1 commit into from
Jan 10, 2025

Conversation

anund
Copy link
Contributor

@anund anund commented Jan 10, 2025

Extends vim validation and file type detection to theme files

cc @gpanders @beaumccartney (as you were involved on the previous vim PR)

src/config/vim.zig Outdated Show resolved Hide resolved
\\if exists("current_compiler")
\\ finish
\\endif
\\let current_compiler = "ghostty"
\\
\\CompilerSet makeprg=ghostty\ +validate-config
\\CompilerSet makeprg=ghostty\ +validate-config\ --config-file='%:p'
\\CompilerSet errorformat=%f:%l:%m
Copy link
Member

Choose a reason for hiding this comment

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

I didn't take a closer look at this before, but is this efm actually correct? When I run ghostty +validate-config with an invalid option it does not print out the filename or line number, only the message. In which case 'errorformat' should just be %m. If validate-config ever does print the filename and line number in filename:line:message format, then this would be correct.

Copy link
Contributor Author

@anund anund Jan 10, 2025

Choose a reason for hiding this comment

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

It's mostly correct. Ghostty has ~3 or more output formats depending on where an error happens. You should see one of the three follow forms:

  • /home/<user>/.config/ghostty/themes/testing:4:not-a-field: unknown field
  • <file>:<number>: <message>"(looking at possibilities in diagnostic.zig.write, may not be possible atm)
  • <message>

Those examples can go wrong in the current code:

  • not-a-field: unknown field (location = none, anytime a key is replayed)
  • error opening config-file /home/<user>/.config/ghostty/themes/nope: error.FileNotFound (non-reproducible steps generated in loadRecursiveFiles from originally fixed keys)

And cli output:

  • cli:4:palette: invalid value "not right" (not technically a worry here)

Copy link
Contributor Author

@anund anund Jan 10, 2025

Choose a reason for hiding this comment

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

Suggested change
\\CompilerSet errorformat=%f:%l:%m
\\CompilerSet errorformat=%f:%l:%m
\\CompilerSet errorformat+=%m

I think will work but I was unsure where to verify that second step in vim. The %m output was ignored by :cw when tested.

Copy link
Member

Choose a reason for hiding this comment

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

Those examples can go wrong in the current code:

  • not-a-field: unknown field (location = none, anytime a key is replayed)

Ah that explains it, I was testing it by using an unknown field.

I think will work but I was unsure where to verify that second step in vim.

Rather than two CompilerSets I'd suggest either (1) using a comma to set both in one command: CompilerSet errorformat=%f:%l:%m,%m or (2) leaving it as-is and updating the error handling code in Ghostty to always print the filename and line number (this seems like the best approach to me, personally, but I haven't looked into how difficult that'd be).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll do (1) for this PR. (2) is something I've been poking at. It's just taking a while to unwind the current code.

@anund anund force-pushed the vim_compiler_improvements branch 2 times, most recently from 791e889 to fe47aa4 Compare January 10, 2025 15:23
@anund anund force-pushed the vim_compiler_improvements branch from fe47aa4 to c03828e Compare January 10, 2025 15:45
@mitchellh mitchellh merged commit e475560 into ghostty-org:main Jan 10, 2025
24 checks passed
@github-actions github-actions bot added this to the 1.1.0 milestone Jan 10, 2025
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.

4 participants