Resync the lexers with Scintillua

[qiu-x]

Related to #908. I fetched the lexers from Scintillua and also added some changes from the scintillua branch. Please let me know if there are other necessary changes to make this work properly.

[mcepl]

How is this supposed to be in relation to scintillua branch? Should it replace it? Is the top of this patch further than that branch?

[qiu-x]

It is supposed to replace the scintillua branch, since some of the lexers are already out of date there. This is a new resync + some changes pulled from the old branch. If there are lot of complex changes necessary before this can be merged, we could also make a new branch based on this PR.

[mcepl]

How do this relate to https://github.com/martanne/vis/pull/1025 ? That one seems to be ready to merge for me.

[Nomarian]

This one is better, actually respects themes. but no go on the html lexer.

[qiu-x]

but no go on the html lexer

It looks like there is a problem with custom style names like:

lex:add_style('doctype', lexer.styles.comment)

For example the CSS at_rule style also does not work

-- At rule.
lex:add_rule('at_rule', token('at_rule', '@' *
  word_match('charset font-face media page import namespace keyframes')))
lex:add_style('at_rule', lexer.styles.preprocessor)

[qiu-x]

@Nomarian The problem with the html lexer should be resolved by 7e47568. Please let me know if you find any other problems.

[mcepl]

@Nomarian The problem with the html lexer should be resolved by 7e47568. Please let me know if you find any other problems.

Complete nothing:

My current code is https://git.sr.ht/~mcepl/vis/tree/devel, this branch should be fully included (I see specifically https://git.sr.ht/~mcepl/vis/commit/f8a1ffb8959b72e0784f3c7bbd9d74a9ef213fbe).

[qiu-x]

This could be also an issue with your color theme (perhaps some of my changes broke it), but I also found some strange behavior while using your branch(https://git.sr.ht/~mcepl/vis/tree/devel). When I open a file that is in my current working directory the syntax highlighting works fine:

$ vis lol.html

But when I change directories it breaks:

$ mkdir somedir && cd somedir
$ vis ../lol.html

Can you try directly using my branch?

Edit: I replicated the issue on a different PC

[mcepl]

This could be also an issue with your color theme (perhaps some of my changes broke it), but I also found some strange behavior while using your branch(https://git.sr.ht/~mcepl/vis/tree/devel). When I open a file that is in my current working directory the syntax highlighting works fine:

Edit: I replicated the issue on a different PC

Yes, I can reproduce here. So, now, wish me good luck with bisecting!

[mcepl]

I had to screw up bisecting somehow: I’ve got https://github.com/martanne/vis/commit/1ca937518ed5ca00e18e5e59e1fef0d9bd628e16 as the first bad commit, but I really don’t see why it should break anything.

@Nomarian Isn’t there something you want to tell us?

[jpe90]

IIUC outside of cwd, this will not match so filetype extension detection breaks. https://github.com/martanne/vis/blob/1ca937518ed5ca00e18e5e59e1fef0d9bd628e16/lua/plugins/filetype.lua#L491

I think you can just strip the path to fix it.

diff --git a/lua/plugins/filetype.lua b/lua/plugins/filetype.lua
index b8640fc..2e7a412 100644
--- a/lua/plugins/filetype.lua
+++ b/lua/plugins/filetype.lua
@@ -485,6 +485,7 @@ vis.events.subscribe(vis.events.WIN_OPEN, function(win)
    end

    local path = win.file.name -- filepath
+   path =  path:gsub('^.*/', '')
    local mime

    if path and #path > 0 then
-- 
2.32.1 (Apple Git-133)

[mcepl]

@jpe90 Thank you. Seems to help with this commit. I have still some troubles with building proper openSUSE package, but that's my problem, not related to this PR.

[mcepl]

With the return of 1ca937518ed5ca00e18e5e59e1fef0d9bd628e16 and adding @jpe90’s change, I get everything working, except for one thing: @Nomarian, vis again cannot load a lexer from ~/.config/vis/lexers. Any ideas?

[qiu-x]

vis again cannot load a lexer from ~/.config/vis/lexers

That's strange, for me both branches are properly loading the lexers.

[qiu-x]

Also, since it's possible to run vis without specifing a path or filename we probably want something like this:

        local path = win.file.name -- filepath
-       path = path:gsub('^.*/', '')
        local mime

        if path and #path > 0 then
+               path = path:gsub('^.*/', '')
                local name = path:match("[^/]+") -- filename
                if name then
                        local unchanged

[Nomarian]

scratch that, I just woke up.

@qiu-x

You should not change path, file needs it to run when checking for mime-type.

also, a resync with master is needed since its removing some stuff that ninewise merged.

vis again cannot load a lexer from ~/.config/vis/lexers

scite needs to keep the lexers in the global table, this can be fixed by not using LEXERPATH and just putting the vis paths in package.path.

@jpe90

Correct, that fails, missed the $ on my rewrite, I'll blame @ninewise for that, always wants to "make code legible for other people" smh.

Fixed on

https://github.com/martanne/vis/pull/1029/files

[Nomarian]

@qiu-x

It is supposed to replace the scintillua branch, since some of the lexers are already out of date there. This is a new resync + some changes pulled from the old branch. If there are lot of complex changes necessary before this can be merged, we could also make a new branch based on this PR.

If this is supposed to replace the scintilua branch, then this should be pushed to the scintilua branch no?

Right now its master -> master

[mcepl]

If anybody is concerned with the state of the https://github.com/martanne/vis/tree/scintillua branch, then when ignoring just plain syncing with the upstream commits (which we should have had covered), these remains:

• [x] 0158a39 lexers/markdown: support https://foo.bar links in markdown included in 959c4eea
• [X] 759f330 lexers/gemini: remove legacy gemini lexer done in https://github.com/orbitalquark/scintillua/pull/73
• [x] 55f36c9 lexers/dsv: convert to new lexer format I have it here https://git.sr.ht/~mcepl/vis/commit/872ae9d6766afd52a6cfc7fbd1a128fd6345a3f7 if anybody wants it; included in https://github.com/qiu-x/vis/commit/70b0e5e3573071ecde4f8f2f3d7f9d4c4c42fe88
• [X] dc6ac3a themes/zenburn: add some previously missing style attributes this PR has it as https://github.com/martanne/vis/pull/1018/commits/568c2974965a18a77a992c39128ef7791d998bf3
• [x] e3d05d4 lexers: update README for new upstream lexer format
• [X] 405350f lexers/git-rebase: convert to new lexer format done in https://github.com/orbitalquark/scintillua/pull/72
• [X] 85917c5 lexers/strace: convert to new lexer format done in https://github.com/orbitalquark/scintillua/pull/73
• [X] 214f3ac lexers/html: performance improvement https://github.com/orbitalquark/scintillua/pull/17
• [x] b99626c lexers/wsf: performance improvement https://github.com/orbitalquark/scintillua/pull/16
• [x] 63a6593 lexers/xml: performance improvement https://github.com/orbitalquark/scintillua/pull/15
• [ ] 5e6d890 lexers: look up style attributes in property table
• [x] fa13056 vis-lua: make sure default lexer styles are defined
• [x] d9378ef vis-lua: make sure themes define standard colors
• [x] 601bf8e vis-lua: convert table style definitions into string form
• [x] 083d600 lexers: add upper bound on lexing time
• [X] 56ea627 lexers: redirect print function to vis:info() this PR has it as https://github.com/martanne/vis/pull/1018/commits/7d13e88b933915dbbf533e785d607f7c6cf8c378
• [x] 0df307f lexers: load lexers from sub directory
• [x] d35a770 lexers: make LPeg module global
• [x] 2a22194 lexers: update lexer.lua to upstream scintillua version

[mcepl]

vis again cannot load a lexer from ~/.config/vis/lexers

That's strange, for me both branches are properly loading the lexers.

Yes, forget about it, I cannot reproduce it anymore.

[qiu-x]

Those are also included in this PR:

• [ ] e3d05d4 lexers: update

in cd64f6d

• [ ] fa13056 vis-lua: make sure default lexer styles are defined
• [ ] d9378ef vis-lua: make sure themes define standard colors
• [ ] 601bf8e vis-lua: convert table style definitions into string form
• [ ] 0df307f lexers: load lexers from sub directory
• [ ] d35a770 lexers: make LPeg module global

in a8ad8add

And I don't think that this is necessary anymore:

• [ ] 5e6d890 lexers: look up style attributes in property table

[qiu-x]

If this is supposed to replace the scintilua branch, then this should be pushed to the scintilua branch no?

Actually, what I meant by "replace" was that this is a new attempt at syncing with scintilua, so we can make a new branch out of this PR or if possible, directly merge into master (when all issues are resolved).

[mcepl]

If this is supposed to replace the scintilua branch, then this should be pushed to the scintilua branch no?

Actually, what I meant by "replace" was that this is a new attempt at syncing with scintilua, so we can make a new branch out of this PR or if possible, directly merge into master (when all issues are resolved).

+1 for merging into master. We need compatibility to be able to work with the scintillua upstream, and although we shouldn’t have regressions there is no reason why shouldn’t continue fixing any remaining bugs on new topic after merging this one.

Strong -1 to merging into new branch. Separate branch leads to endless improvements which is never good enough for merging. See at the current https://github.com/martanne/vis/tree/scintillua branch: 165 commits, year and half old, and nothing in master.

[mcepl]

Merge branch 'martanne:master' into master

Isn’t it the other way around?

[qiu-x]

Rebased on current master

[mcepl]

• orbitalquark/scintillua#72
• orbitalquark/scintillua#73

Have been both rejected (only lexers for documents, not for output of programs), so we will have to carry strace and git-rebase lexers in our repo.

[Nomarian]

it seems filetype.lua erases some commits but its ok in the branch... weird

also is there a need to recreate searchpath when package.searchpath exists?

[qiu-x]

also is there a need to recreate searchpath when package.searchpath exists?

Not really, the local searchpath just removes some logging that is not useful in this case.

[qiu-x]

it seems filetype.lua erases some commits but its ok in the branch... weird

should be fixed in afc1c8b.

[ninewise]

Is this ready for testing/merging or are there still things to do?

Regardless, could you rebase on current master? I've merged mcepl's work on the github CI so we could get green builds here.

[mcepl]

Is this ready for testing/merging or are there still things to do?

I have been running this for last couple of weeks and I haven’t encountered any disasters. The only unwelcome thing is that we have to carry some of our lexers ourselves (they are included in this PR), because upstream does want to limit themselves only to file formats, so our git-rebase, gemini, and strace lexers were rejected. Aside from this, I think we are all set and we cannot do much about this anyway.

[deepcube]

On Mon Aug 15, 2022 at 6:38 AM MDT, Matěj Cepl wrote:

The only unwelcome thing is that we have to carry some of our lexers ourselves (they are included in this PR), because upstream does want to limit themselves only to file formats, so our git-rebase, gemini, and strace lexers were rejected. Aside from this, I think we are all set and we cannot do much about this anyway.

How difficult would it be to put our custom lexers in a separate directory? That should make it easier to do updates in the future and open up the possibility of a submodule or subtree merges.

[qiu-x]

How difficult would it be to put our custom lexers in a separate directory?

I think it would be rather trivial. I was planning to do another PR with that change.

[mcepl]

I think it would be rather trivial. I was planning to do another PR with that change.

+1 on that. Could we merge this first and then improve it?

[ninewise]

I'm moving to this branch, too. I want to run it for a bit before getting it in master.

[mcepl]

Awesome! Upstream changed her mind and they will include all our lexers https://github.com/orbitalquark/scintillua/pull/74

[qiu-x]

I'm moving to this branch, too. I want to run it for a bit before getting it in master.

How is it going? Are there any problems with the patch so far?

[mcepl]

Could you perhaps also send the patch to the mailing list for further comments - not sure how many people follow development here on github.

Could we actually make a decision and clarify the “official” position on GitHub vis-à-vis Sourcehut. There are many tickets here and then there are submissions on sr.ht (e.g., https://todo.sr.ht/~martanne/vis). I would love to see GitHub closed, but perhaps we should admit the defeat and use only that? Whatever, but some decision should be made, in my opinion.

[qiu-x]

Could you perhaps also send the patch to the mailing list for further comments

Sure, I will send it later today.

Could we actually make a decision and clarify the “official” position on GitHub vis-à-vis Sourcehut. There are many tickets here and then there are submissions on sr.ht (e.g., https://todo.sr.ht/~martanne/vis). I would love to see GitHub closed, but perhaps we should admit the defeat and use only that? Whatever, but some decision should be made, in my opinion.

I think that closing GitHub entirely is not a good idea, since it would negatively impact the discoverability of the project. SourceHut is a much more pleasant to use platform, so we could use that as the main hosting and GitHub as a mirror.

[mcepl]

I think that closing GitHub entirely is not a good idea, since it would negatively impact the discoverability of the project. SourceHut is a much more pleasant to use platform, so we could use that as the main hosting and GitHub as a mirror.

I was thinking more about switching off issue tracker and pull requests.

[mcepl]

Those are also included in this PR: And I don't think that this is necessary anymore:

• [ ] 5e6d890 lexers: look up style attributes in property table

@qiu-x , care to elaborate?

https://github.com/qiu-x/vis/blob/c3fe2571a4b7350a8256b2b16c90b8366aafd115/lua/lexers/lexer.lua#L872

seems a bit hackish to me, approach in that commit seems more sophisticated.

[ninewise]

merged in 8a420ecc4c1ed50111464ec66901bd983eaf2dbd, thanks for the effort and patience!

[mcepl]

@qiu-x I hoped that after merging this, this diff would be more or less empty. What did I miss?

[qiu-x]

@qiu-x I hoped that after merging this, this diff would be more or less empty. What did I miss?

It's because there were already quite a few changes in Scintillua. Try diffing with commit 9088723504b19f8611b66c119933a4dc7939d7b8 on Scintillua. I guess it's time for another resync ;)

[qiu-x]

@qiu-x , care to elaborate? https://github.com/qiu-x/vis/blob/c3fe2571a4b7350a8256b2b16c90b8366aafd115/lua/lexers/lexer.lua#L872 seems a bit hackish to me, approach in that commit seems more sophisticated.

IIRC, I did it like this because I wanted to have as few changes to lexer.lua as possible, but I can look into this in a future resync.

[mcepl]

IIRC, I did it like this because I wanted to have as few changes to lexer.lua as possible, but I can look into this in a future resync.

I think so, if you can make one this time hopefully a quick resync (and we persuade @ninewise to merge it), we could then submit some PRs to scintillua directly.