search

martanne / vis

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

Support old option names and don't deprecate them #1127

Closed hazen2215 closed 9 months ago

hazen2215 commented 9 months ago

Changing option names is understandable because it may be useful for writing lua plugins but deprecating old names and adding new codes to detect deprecated options seems unnecessary to me. vis supports alternative option names and the second commit of this pr just adds old names to alternatives.

I will squash two commits if desired.

rnpnr commented 9 months ago

Why? If it was up to me I would delete all the aliases. These ones are particularly useless in my view.

Also only 3 lines of code were added. The rest will be deleted after 0.9 is released.

hazen2215 commented 9 months ago

Noticed the deprecation message of 'show-eof' option when using vis on master branch because I have vis:command('set show-eof off') on my visrc.lua. Until most distros update vis to 0.9 and I can finally use vis.options.xxx, I'm stuck with :set command and seeing such message every time is quite annoying Also I haven't see config breaking change like this on vis other than scintilua update, but that only broke local lexers and syncing visrc.lua among different versions of vis had previously no problems.

rnpnr commented 9 months ago

If you are seeing that message you can use the supported name instead.

Also I haven't see config breaking change like this on vis other than scintilua update

Hence why it is being marked as deprecated and supported for a release. Those options were added before vis had a lua interface and so the names were chosen without support for lua in mind.

hazen2215 commented 9 months ago

OK I found the plugin (https://github.com/milhnl/vis-options-backport) backporting vis.options to older versions that addresses renaming problem. So if the maintainers want to I'm ok with closing this PR since the solution is found. But also I'd like to hear others' opinion if consistency is good reason for deprecation and opinion about deprecating something that's been around for some time.

hazen2215 commented 9 months ago

also quote https://github.com/martanne/vis/issues/1001#issuecomment-1116369265

It will be a very slow, careful process. We don't want to make things worse, and we don't have the intricate knowledge of the internals that martanne had. Please do be patient as we learn the ropes.

TwoF1nger commented 9 months ago

But also I'd like to hear others' opinion if consistency is good reason for deprecation and opinion about deprecating something that's been around for some time.

vis is hardly a household name. I'd say break everything while you can and don't even bother with deprecation notices.

hazen2215 commented 9 months ago

Whether vis is "hardly" household name with 4K stars is debatable but vis is certainly well known to people who prefer simplicity or "suckless" or "KISS" or whatever Lately vis is getting lots of commits by incorporating many old PRs since new maintainers are added and while getting the bugs fixed is nice I've seen some projects start making many breaking changes after BDFL(s) disappearance/handover (like anything.el/helm) and I'm concerned about vis going that way.

mcepl commented 9 months ago

Lately vis is getting lots of commits by incorporating many old PRs since new maintainers are added and while getting the bugs fixed is nice I've seen some projects start making many breaking changes after BDFL(s) disappearance/handover (like anything.el/helm) and I'm concerned about vis going that way.

Quite certainly we will make changes which martanne would not make, for one, we don’t have a crystal ball to read his mind. However, there are some reasons I don’t expect any radical changes in the direction different from what vis does right now:

  1. We don’t have strength to do much. Current rather strong stream of changes is mostly just processing already existing and neglected in PR queue (there are some PRs created by @rnpnr lately, but these are mostly small fixes of existing issues), and I think we just cannot make any radical changes (which is one of unmentioned reasons behind my negative attitude towards suggested changes to treesitter in https://github.com/martanne/vis/issues/668). See how much time we need just with updating vis to follow upstream released highlighters from scintillua.
  2. Given that there is nobody with sufficiently large amount of time, there is nobody who can claim to be BFDL, so there is nobody with large enough authority to decide about radical changes. Decision “to do whatever we were doing before” is usually the only one we can agree upon when dealing with any conflict (a possible conflict, I don’t think we had some great conflict so far).
  3. I don’t actually see any reason why to make radical changes in vis. The idea of small C editor with large number of Lua plugins (which anybody can create and modify as they wish them to be) seems like a good design, and I don’t think there is anything which need to be changed.
rnpnr commented 9 months ago

+1 to what @mcepl is saying

Since vis no longer has a "BDFL" the review and application of patches has taken more of a community driven approach. If you are concerned about the direction vis is going feel free to make your opinion heard about any patches here or on the mailing list. Any commits that are not minor are given some amount of time to sit and give people a chance to comment. That includes major commits by me (the lua option patch sat around for around a month before I applied it). I have been applying bug fixes and minor things at a faster rate but they are still being filtered.

In any case this pull request doesn't fix any of this so I think it should be closed. I don't think keeping such an alias around because "thats how it used to be" is a good programming practice.