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

Release vis-0.9 #1115

Closed
5 of 8 tasks
rnpnr opened this issue Aug 1, 2023 · 31 comments
Closed
5 of 8 tasks

Release vis-0.9 #1115

rnpnr opened this issue Aug 1, 2023 · 31 comments

Comments

@rnpnr
Copy link
Collaborator

rnpnr commented Aug 1, 2023

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
Copy link
Contributor

erf commented Aug 1, 2023

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

@rnpnr
Copy link
Collaborator Author

rnpnr commented Aug 1, 2023

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
Copy link
Contributor

mcepl commented Aug 1, 2023

You should add martanne/vis-test#12

@mcepl
Copy link
Contributor

mcepl commented Aug 1, 2023

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

@rnpnr
Copy link
Collaborator Author

rnpnr commented Aug 2, 2023

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
Copy link
Contributor

mcepl commented Aug 4, 2023

Also, #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
Copy link
Collaborator

ninewise commented Aug 7, 2023

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

@mcepl
Copy link
Contributor

mcepl commented Aug 7, 2023

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

Did you? Where?

@bartgrantham
Copy link

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
Copy link
Contributor

mcepl commented Aug 8, 2023

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.

@rnpnr rnpnr pinned this issue Aug 16, 2023
@mcepl
Copy link
Contributor

mcepl commented Aug 28, 2023

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

@rnpnr
Copy link
Collaborator Author

rnpnr commented Sep 3, 2023

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
Copy link
Contributor

mcepl commented Sep 3, 2023

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 orbitalquark/scintillua#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
Copy link
Contributor

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
Copy link
Collaborator Author

rnpnr commented Sep 5, 2023

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
Copy link
Contributor

mcepl commented Sep 5, 2023

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
Copy link
Contributor

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
Copy link
Contributor

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

@mcepl
Copy link
Contributor

mcepl commented Sep 6, 2023

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
Copy link
Collaborator Author

rnpnr commented Sep 6, 2023 via email

@mcepl
Copy link
Contributor

mcepl commented Sep 6, 2023

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
Copy link
Collaborator Author

rnpnr commented Sep 13, 2023

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
Copy link
Contributor

mcepl commented Sep 13, 2023

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
Copy link
Collaborator Author

rnpnr commented Sep 15, 2023

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
Copy link
Contributor

mcepl commented Sep 16, 2023

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
Copy link
Contributor

No vis-0.9 release this year (2023)?

@rnpnr
Copy link
Collaborator Author

rnpnr commented Dec 29, 2023

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
Copy link

we do have quite a number of improvements over 0.8

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

@Eolien55
Copy link

Eolien55 commented Apr 7, 2024

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
Copy link
Contributor

mcepl commented Apr 9, 2024

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
Copy link
Collaborator Author

rnpnr commented Apr 30, 2024

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.

@rnpnr rnpnr closed this as completed May 2, 2024
@rnpnr rnpnr unpinned this issue May 2, 2024
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

No branches or pull requests

9 participants