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

runtime(stylus): include syntax and ftplugin #14656

Closed
wants to merge 21 commits into from

Conversation

pheiduck
Copy link
Contributor

astro depend on this plugin, so bring that upstream too.
@wavded
@tpope
@lepture

Copy link
Member

@chrisbra chrisbra left a comment

Choose a reason for hiding this comment

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

I have no idea what the src/testdir/stylus/test.styl file is for, I don't think it belongs here. If you want to create a syntax test, please have a look at runtime/syntax/testdir instead.

runtime/syntax/stylus.vim Outdated Show resolved Hide resolved
runtime/indent/stylus.vim Outdated Show resolved Hide resolved
runtime/indent/stylus.vim Outdated Show resolved Hide resolved
@pheiduck pheiduck changed the title runtime(stylus): include syntax, ftplugin and test runtime(stylus): include syntax and ftplugin Apr 28, 2024
@pheiduck pheiduck requested a review from chrisbra April 28, 2024 18:06
runtime/ftplugin/stylus.vim Outdated Show resolved Hide resolved
runtime/ftplugin/stylus.vim Outdated Show resolved Hide resolved
runtime/indent/stylus.vim Outdated Show resolved Hide resolved
runtime/indent/stylus.vim Outdated Show resolved Hide resolved
runtime/syntax/stylus.vim Outdated Show resolved Hide resolved
runtime/syntax/stylus.vim Outdated Show resolved Hide resolved
@chrisbra
Copy link
Member

Did you check with the maintainers, if they are fine with including it here? In general, it looks like this still requires a bit of more work.

@pheiduck
Copy link
Contributor Author

pheiduck commented Apr 28, 2024

Did you check with the maintainers, if they are fine with including it here? In general, it looks like this still requires a bit of more work.

I pinged all, but if we get no response I desided to not include this.
I opend an Issue at Maintainers Repo and send him an email lets see if I get a response.

@pheiduck pheiduck requested a review from chrisbra April 28, 2024 20:11
runtime/filetype.vim Outdated Show resolved Hide resolved
runtime/ftplugin/stylus.vim Show resolved Hide resolved
runtime/ftplugin/stylus.vim Show resolved Hide resolved
runtime/ftplugin/stylus.vim Show resolved Hide resolved
runtime/indent/stylus.vim Outdated Show resolved Hide resolved
@chrisbra
Copy link
Member

What is the issue with all those css syntax words? Shouldn't that belong to a separate css syntax script and be sourced for stylus files?

@pheiduck
Copy link
Contributor Author

pheiduck commented Apr 29, 2024

What is the issue with all those css syntax words? Shouldn't that belong to a separate css syntax script and be sourced for stylus files?

Not sure, if this is possible to remove without breaking effects.
Revised the Readme its bundled with HTML5/CSS3 so maybe save to get rid of.

@pheiduck pheiduck marked this pull request as draft April 29, 2024 13:33
@wavded
Copy link

wavded commented Apr 29, 2024

I'm happy to have stylus.vim be officially supported in VIM. I am quite removed from the project nowadays so I am not up to speed on any best practices that should be employed. The CSS3 file was originally added because the VIM packages at that time were missing a lot of more recent CSS definitions. It is likely that this package can just depend on the bundled CSS syntax without needing that file anymore.

@pheiduck pheiduck requested a review from chrisbra April 29, 2024 15:42
runtime/ftplugin/stylus.vim Outdated Show resolved Hide resolved
runtime/ftplugin/stylus.vim Outdated Show resolved Hide resolved
runtime/ftplugin/stylus.vim Outdated Show resolved Hide resolved
runtime/indent/stylus.vim Outdated Show resolved Hide resolved
src/testdir/test_filetype.vim Outdated Show resolved Hide resolved
@chrisbra
Copy link
Member

I don't know stylus and I don't know how much it depends on decent html and or CSS support. But I would think, if it requires those syntax to be present, it should 'source/runtime' those syntax files specifically, instead of declaring them all on its own.

runtime/ftplugin/stylus.vim Outdated Show resolved Hide resolved
runtime/ftplugin/stylus.vim Outdated Show resolved Hide resolved
runtime/ftplugin/stylus.vim Outdated Show resolved Hide resolved
@pheiduck pheiduck requested a review from chrisbra April 30, 2024 15:21
runtime/indent/stylus.vim Outdated Show resolved Hide resolved
@chrisbra
Copy link
Member

can you please make sure, it doesn't error out before sending updated commits here?

@chrisbra
Copy link
Member

chrisbra commented May 1, 2024

okay, I think looks good now

@pheiduck pheiduck marked this pull request as ready for review May 1, 2024 14:51
@chrisbra
Copy link
Member

chrisbra commented May 1, 2024

okay, only thing I noticed was missing to reset formatoptions in b:undo_ftplugin. I can fix this during merge.

@chrisbra chrisbra closed this in 2d919d2 May 1, 2024
clason added a commit to clason/neovim that referenced this pull request May 1, 2024
Problem:  filetype: stylus files not recognized
Solution: Detect '*.styl' and '*.stylus' as stylus filetype,
          include indent, filetype and syntax plugin
          (Philip H)

closes: vim/vim#14656

vim/vim@2d919d2

Co-authored-by: Philip H <47042125+pheiduck@users.noreply.github.com>
clason added a commit to neovim/neovim that referenced this pull request May 1, 2024
Problem:  filetype: stylus files not recognized
Solution: Detect '*.styl' and '*.stylus' as stylus filetype,
          include indent, filetype and syntax plugin
          (Philip H)

closes: vim/vim#14656

vim/vim@2d919d2

Co-authored-by: Philip H <47042125+pheiduck@users.noreply.github.com>
phanen pushed a commit to phanen/neovim that referenced this pull request May 3, 2024
Problem:  filetype: stylus files not recognized
Solution: Detect '*.styl' and '*.stylus' as stylus filetype,
          include indent, filetype and syntax plugin
          (Philip H)

closes: vim/vim#14656

vim/vim@2d919d2

Co-authored-by: Philip H <47042125+pheiduck@users.noreply.github.com>
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.

None yet

3 participants