search

martanne / vis

A vi-like editor based on Plan 9's structural regular expressions
Other
4.19k stars 259 forks source link

Release vis-0.9 #1115

Closed rnpnr closed 2 months ago

rnpnr commented 11 months ago

It has been a while since the last release and there have been a number of bug fixes and new features. This came up on the mailing list and I think we are about ready for a new release.

There are a few outstanding items I was thinking of completing first:

Suggestions for anything else?

erf commented 11 months ago

Maybe review this issue regarding "Prevent flickering in curses".

rnpnr commented 11 months ago

Sure I can look at that one. I don't use window transparency though so it would better if someone who cares about it tests and indicates if there is a regression.

mcepl commented 11 months ago

You should add https://github.com/martanne/vis-test/pull/12

mcepl commented 11 months ago

And we should probably add https://github.com/martanne/vis-test/pull/29 and https://github.com/martanne/vis-test/pull/22, although that may wait on the merge of vis-test repo into the main one.

rnpnr commented 11 months ago

Unfortunately I am unable to merge things in that repo. If they aren't applied by someone else I will make sure they are applied here after moving the test repo.

mcepl commented 11 months ago

Also, https://github.com/martanne/vis/pull/945 … it looks like something so simple that it could be easily approved or rejected, if reviewed by somebody who actually knows what she is talking about.

ninewise commented 10 months ago

I merged the mentioned tests, so we can have a clean history to join the repositories after release.

mcepl commented 10 months ago

I merged the mentioned tests, so we can have a clean history to join the repositories after release.

Did you? Where?

bartgrantham commented 10 months ago

I'd like to request that updates to the docker build mechanism be included in this release, if it isn't already. It's very useful to be able to build a fully-featured, static, current version of vis to quickly deploy to a new machine. Come to think of it, maybe I should also request that as a release artifact for 0.9: a "no install" static binary, at least for Linux x64 and arm.

Also, I don't know how many people use vis on macOS, but it'd be nice if 0.9 was available via brew/macports without too much delay. I assume that would happen anyways, I just wanted to voice my vote for macOS support.

mcepl commented 10 months ago

I'd like to request that updates to the docker build mechanism be included in this release, if it isn't already. It's very useful to be able to build a fully-featured, static, current version of vis to quickly deploy to a new machine. Come to think of it, maybe I should also request that as a release artifact for 0.9: a "no install" static binary, at least for Linux x64 and arm.

I am eagerly waiting on your PR for https://github.com/martanne/vis/blob/master/Dockerfile.

Also, I don't know how many people use vis on macOS, but it'd be nice if 0.9 was available via brew/macports without too much delay. I assume that would happen anyways, I just wanted to voice my vote for macOS support.

This project has absolutely nothing to do with Brew, it seems their page is https://formulae.brew.sh/formula/vis and if you use it, then you probably know how to contact its owners.

mcepl commented 10 months ago

To the synchronizing with scintillua we have also https://github.com/orbitalquark/scintillua/issues/99 there is something weird going on with lex:add_style and I am not sure what.

rnpnr commented 10 months ago

I wonder if I should just apply @mcepl's patch for updating the lexers to scintillua 6.2. There is still an issue with the performance of the bash lexer but I can just comment out that part of the code. Its a feature that isn't supported in the current lexers so we aren't downgrading anything. There is likely going to be some fall out in some plugins. For example I know that the syntax aware part of the vis-spellcheck plugin no longer works. I would like for them to have a chance to update before the release.

Note the patch upgrading lexers was supposed to be sent to the mailing list but sourcehut garbled it somehow. Matej emailed me a copy and its sitting in my tree. You can also see it here: https://git.sr.ht/~mcepl/vis/log/devel_scintillua

mcepl commented 10 months ago

I wonder if I should just apply @mcepl's patch for updating the lexers to scintillua 6.2. There is still an issue with the performance of the bash lexer but I can just comment out that part of the code. Its a feature that isn't supported in the current lexers so we aren't downgrading anything. There is likely going to be some fall out in some plugins. For example I know that the syntax aware part of the vis-spellcheck plugin no longer works. I would like for them to have a chance to update before the release.

The problem with https://github.com/orbitalquark/scintillua/issues/99 is that many types of texts are now broken. E.g., vis /tmp/patch.patch shows exactly zero colouring, because it is defined via construct like:

lex:add_rule('addition', token('addition', S('>+') * lexer.any^0))
lex:add_style('addition', {fore = lexer.colors.green})

and 

.../lexers $ grep -l 'lex:add_style' *|wc -l
56
 $
fischerling commented 10 months ago

There is likely going to be some fall out in some plugins. For example I know that the syntax aware part of the vis-spellcheck plugin no longer works. I would like for them to have a chance to update before the release.

One problematic change for vis-spellcheck is the removal of the global lexer cache in 2c0d990. The syntax aware spellchecking works by wrapping the lex function of the lexer used for syntax highlighting. Without the cache lexers.load always returns a new table and therefore it is no longer possible to modify the lexer used in vis-std.lua for syntax highlighting.

A simple way to spellcheck files with a specified syntax is to disable syntax awareness.

local spellcheck = require('plugins/vis-spellcheck')
spellcheck.disable_syntax_awareness = true

Disabling the syntax aware spellchecking will always cause the full viewport to be highlighted.

rnpnr commented 10 months ago

E.g., vis /tmp/patch.patch shows exactly zero colouring, because it is defined via construct like:

The diff.lua lexer is updated so it isn't defined like that anymore. The problem is that now all the themes need to be updated to include lines like

lexers.STYLE_ADDITION = 'fore:green'
lexers.STYLE_DELETION = 'fore:red'

which is quite annoying. You are correct however that the unmigrated legacy lexers do not show syntax highlighting anymore e.g. rest.lua.

One problematic change for vis-spellcheck is the removal of the global lexer cache in 2c0d990.

Good catch. I think that also explains why lexing some files is significantly slower than before.

I think we might need rethink a little about updating to scintillua 6+. Some of these issues are making the user experience much worse.

mcepl commented 10 months ago

One problematic change for vis-spellcheck is the removal of the global lexer cache in 2c0d990.

Good catch. I think that also explains why lexing some files is significantly slower than before.

We should probably open this question with @orbitalquark , shouldn’t we? We cannot be the only ones who are affected by this, right? Do you have any hard data about the speed (yes, of course, vis-spellcheck needs to be ported to the new base).

I think we might need rethink a little about updating to scintillua 6+. Some of these issues are making the user experience much worse.

Just to the contrary, I am all for following scintillua more closely, after 6.* merge, I plan to follow upstream HEAD in my personal repo.

fischerling commented 9 months ago

One problematic change for vis-spellcheck is the removal of the global lexer cache in 2c0d990.

Good catch. I think that also explains why lexing some files is significantly slower than before.

We should probably open this question with @orbitalquark , shouldn’t we? We cannot be the only ones who are affected by this, right? Do you have any hard data about the speed (yes, of course, vis-spellcheck needs to be ported to the new base).

Currently I see no way, how vis-spellcheck's syntax awareness could be ported without changes to either vis-std.lua or lexer.lua. Because we need to get the token stream to only highlight misspelled words within the checked tags or ideally directly modify the token stream.

Both is not easily possible without wrapping the lex function of the lexer table used by vis-std.lua.

Retrieving the token stream without hooking into the lexer used by vis-std.lua would require a second lexing run.

If anybody has a good idea how to solve this in a plugin I am open for inspiration and contributions ;)

fischerling commented 9 months ago

We should probably revert https://github.com/orbitalquark/scintillua/commit/6ddc53bc5cece651a17c8cd977258fc0188f56bb when merging the upstream changes. This would at least solve the vis-spellcheck problem.

And the changes look not appropriate for vis anyway since we still use the cache parameter in vis-std.lua when highlighting (https://git.sr.ht/~mcepl/vis/tree/master/item/lua/vis-std.lua#L75).

mcepl commented 9 months ago

We should probably revert orbitalquark/scintillua@6ddc53b when merging the upstream changes. This would at least solve the vis-spellcheck problem.

And the changes look not appropriate for vis anyway since we still use the cache parameter in vis-std.lua when highlighting (https://git.sr.ht/~mcepl/vis/tree/master/item/lua/vis-std.lua#L75).

Could you please create patch and send it to me (or at least create a branch with those commits somewhere, so that I could pull from it), please?

rnpnr commented 9 months ago

Could you please create patch and send it to me (or at least create a branch with those commits somewhere, so that I could pull from it), please?

I won't stop you but I think it would be better to look into implementing the caching on vis' side instead. This seems to be the expectation from upstream.

I'm just a little busy this week otherwise I would already be working on it myself.

mcepl commented 9 months ago

I'm just a little busy this week otherwise I would already be working on it myself.

That’s the problem. Me too (returned after two weeks of vacation and short sickness).

rnpnr commented 9 months ago

I have re-implemented the caching locally for now and the spellcheck plugin works just fine without any modifications. You can find that change in my rnpnr dev branch. The default themes still need to be updated. I will try to work on it when I can. The bash lexer is still horribly slow but I made a test script for measuring this which I will add to my report upstream.

mcepl commented 9 months ago

Can we actually merge in those tests? I don’t think it makes any difference in functionality or anything, it only makes my git repo a mess, so I would like to see it merged or I will have to restore that silly submodule.

rnpnr commented 9 months ago

Can we actually merge in those tests?

Originally I thought it would be better do one last release with them separate but maybe that doesn't really matter?

mcepl commented 9 months ago

Originally I thought it would be better do one last release with them separate but maybe that doesn't really matter?

It doesn’t. On the other hand, I have just reverted that change in my repo and pushed it to a branch (merge_tests), so I really don’t care know. I think I have really persuaded myself that the change is truly NOOP as a result, so it can go in now, but do whatever you wish.

Disonantemus commented 6 months ago

No vis-0.9 release this year (2023)?

rnpnr commented 6 months ago

No vis-0.9 release this year (2023)?

Seems unlikely. Unless people are happy with what we have right now (we do have quite a number of improvements over 0.8).

The outstanding item in my mind is #1154 which still has some issues with terminal attributes. To me that is the main blocker for #1133. Once #1154 is fixed up and we have a base theme most of the missing theming in #1133 can be fixed. But both of these could be pushed back until next year if people want a release.

goyalyashpal commented 2 months ago

we do have quite a number of improvements over 0.8

so, how about a nightly for the non-builders out there 😅

Eolien55 commented 2 months ago

Now that #1154 and #1133 are both merged, is the last milestone for 0.9 #953? If it's not maybe it'd be worth editing the opening text of the issue?

mcepl commented 2 months ago

we do have quite a number of improvements over 0.8

so, how about a nightly for the non-builders out there 😅

What about them? Are you volunteering? Which platform (OS/distro)?

I maintain (and from time to time update) openSUSE ones at https://build.opensuse.org/package/show/home:mcepl/vis

rnpnr commented 2 months ago

I made a PR for the release: #1185 Let me know if there are any issues.

Now that #1154 and #1133 are both merged, is the last milestone for 0.9 #953? If it's not maybe it'd be worth editing the opening text of the issue?

I'm purposefully skipping that for now. There are still a few quirks that need to be ironed out.