vis icon indicating copy to clipboard operation
vis copied to clipboard

Release vis-0.9

Open rnpnr opened this issue 11 months ago • 31 comments

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:

  • [ ] #953
    • Needs a comment on a commit. It's not clear what it's doing.
  • [x] #1113
    • Needs someone else to review
  • [x] #914
    • I can probably just merge this
  • [x] #946
    • Maybe. Needs review in more detail.
  • [x] #675
    • Would be nice to have so that more people will use it and find any bugs. It still has a couple issues that need to be fixed.
    • Applied in 0b01532.
  • [x] #1061
    • Ideally someone who cares about transparency should test and indicate if there is a regression.
    • Applied in d1f2c27.
  • [ ] resync scintillua lexers
    • I think this is probably appropriate before each release.
    • See https://github.com/martanne/vis/issues/961#issuecomment-1673915226
    • In progress: #1133
  • [ ] update changelog
    • I can do this, it shouldn't be difficult.

Suggestions for anything else?

rnpnr avatar Aug 01 '23 02:08 rnpnr

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

erf avatar Aug 01 '23 08:08 erf

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.

rnpnr avatar Aug 01 '23 12:08 rnpnr

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

mcepl avatar Aug 01 '23 21:08 mcepl

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.

mcepl avatar Aug 01 '23 21:08 mcepl

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.

rnpnr avatar Aug 02 '23 01:08 rnpnr

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.

mcepl avatar Aug 04 '23 07:08 mcepl

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

ninewise avatar Aug 07 '23 19:08 ninewise

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

Did you? Where?

mcepl avatar Aug 07 '23 19:08 mcepl

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.

bartgrantham avatar Aug 08 '23 09:08 bartgrantham

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 avatar Aug 08 '23 09:08 mcepl

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.

mcepl avatar Aug 28 '23 11:08 mcepl

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

rnpnr avatar Sep 03 '23 16:09 rnpnr

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
 $ 

mcepl avatar Sep 03 '23 18:09 mcepl

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.

fischerling avatar Sep 05 '23 09:09 fischerling

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.

rnpnr avatar Sep 05 '23 11:09 rnpnr

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.

mcepl avatar Sep 05 '23 17:09 mcepl

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 avatar Sep 06 '23 09:09 fischerling

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).

fischerling avatar Sep 06 '23 09:09 fischerling

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?

mcepl avatar Sep 06 '23 12:09 mcepl

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.

rnpnr avatar Sep 06 '23 13:09 rnpnr

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).

mcepl avatar Sep 06 '23 13:09 mcepl

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.

rnpnr avatar Sep 13 '23 17:09 rnpnr

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.

mcepl avatar Sep 13 '23 20:09 mcepl

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?

rnpnr avatar Sep 15 '23 17:09 rnpnr

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.

mcepl avatar Sep 16 '23 06:09 mcepl

No vis-0.9 release this year (2023)?

Disonantemus avatar Dec 29 '23 10:12 Disonantemus

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.

rnpnr avatar Dec 29 '23 16:12 rnpnr

we do have quite a number of improvements over 0.8

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

goyalyashpal avatar Apr 06 '24 01:04 goyalyashpal

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?

Eolien55 avatar Apr 07 '24 06:04 Eolien55

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

mcepl avatar Apr 09 '24 13:04 mcepl