-
Notifications
You must be signed in to change notification settings - Fork 13
use restart, kill-timeout=0, return all group info #12
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
base: master
Are you sure you want to change the base?
Conversation
akinomyoga
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your contribution.
Could you add the new version as a separate file github302-perlre-server2.bash? The reason that I added the original version in blesh-contrib is to provide an example implementation for bgproc. The original one was simple enough to serve as an example. The new version is more complicated and doesn't seem to be a good example to explain the usage of bgproc.
I would like to keep at least one simple version. Nevertheless, it is good to keep the actually used version, so we can keep two versions.
config/github302-perlre-server.bash
Outdated
| ble-import util.bgproc | ||
|
|
||
| function ble/contrib/config:github302/perlre-server { | ||
| # We redirect 2>/dev/tty here to ensure errors show up |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment is useful for we can still apply this change to the simple version (github302-perlre-server.bash)
Or if the purpose to include it in blesh-contrib is to use it from another module, maybe I'd suggest to include the code as a part of that module. Or if you think this is general enough, we can put it in a new subdirectory like |
|
On Thu, Apr 13, 2023 at 12:48 PM Koichi Murase ***@***.***> wrote:
This is the version my clone of
chaoren/vim-wordmotion uses (I'm hoping to submit that to contrib
somewhere soon).
Or if the purpose to include it in blesh-contrib is to use it from another
module, I'd suggest to include the code in the module.
Since it's already split off as a module and is in theory independently
usable I'd sort of like to keep it as module. But it can be integrated if
you don't want the clutter of two modules.
Britton
… Message ID: ***@***.***>
|
|
I put in https://github.com/bkerin/blesh-contrib a version with the
more complete perlre-server2 renamed and the original restored, and
also a preliminary version of config/vim-wordmotion-ish.bash
The former is hopefully acceptable or close to acceptable as is.
The latter is more a proposition. It still needs everything renamed.
Also I think all the binding should be grouped at the end of something
(the original vim-wordmotion makes these bindings automatically and I
find it convenient).
If I understand your naming convention correctly it is about like this:
_private_var
namespace/subspace/#public_method
namespace/subspace/.private_method
but any particular advice about how things should be named in e.g.
config/vim-wordmotion-ish.bash would be welcome as I'm not entirely
clear on it particularly how alternate widgets should be named. Or
any other advice on how best to do anything.
Britton
…On Thu, Apr 13, 2023 at 12:48 PM Koichi Murase ***@***.***> wrote:
This is the version my clone of
chaoren/vim-wordmotion uses (I'm hoping to submit that to contrib somewhere soon).
Or if the purpose to include it in blesh-contrib is to use it from another module, I'd suggest to include the code in the module.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
I have created a stub PR about the coding style at akinomyoga/ble.sh#317 (the added section can be checked here). I think there are still many things to describe, but I at least wrote about the naming convention for variables and functions.
I actually planned to change styles before merging, but maybe it is a good chance to document all the related styles explicitly. |
|
Nice style guide. I'm unclear about the purpose of the contrib/config versus just contrib. I think maybe contrib is for library-type code and config for things that would alter base behavior? |
I have added a description at the end of CONTRIBUTING § Summary of codebase in akinomyoga/ble.sh#317. I think your guess roughly captures the essential idea. |
I looked at it again, but I think this perl server should be included as a part of the module To make any independent module, I require the functionality to be used from at least two places. Until then, we would maintain the functionality under the module that uses the functionality. This is kind of YAGNI. First, we are not sure if there will actually appear another module that uses the functionality. Second, with only one module using the functionality, it is hard to decide the proper abstraction of the feature; we want more examples to shape a commonly useful interface. Once we make an independent module we wouldn't want to change the interface frequently, so I would like to keep the functionality inside another module and bring up the interface until the day when the functionality leaves the nest to become an independent module. |
Co-authored-by: Koichi Murase <[email protected]>
|
Sounds right. I think I'll make all the name changes per style guide with things as they are, then combine everything. I agree there's little chance perlre-server2 will see other use (bash may get zero-width assertions first :). Of course there's some tension between YAGNI and making existing stuff known. I don't know if anything can be done about this, maybe a list somewhere of independent modules that could be broken out or something (though that could itself be unlikely to be found). For example at the moment I sort of wonder if fzf integration or something has the kind of simple interface to completion I'm interested i right now. |
| # if we wait before sending SIGTERM | ||
| # if we wait before sending SIGTERM. FIXXME: using deferred here would | ||
| # reduce the start-up time cost a tiny bit but then the startup message | ||
| # could be disruptive, and I think they're more valuable at the moment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can consider using ble/util/visible-bell "$message", ble/edit/info/show text "$message", or ble/edit/info/immediate-show text "$message". The latter two are the ones mentioned in akinomyoga/ble.sh#306 (comment).
| # FIXME: I belive the ble.sh author encouraged me to put the perl-re-server2 in | ||
| # it's own file, but for it to get correctly installed it would have to get | ||
| # mentioned hereish | ||
| contrib-srcfiles := $(wildcard contrib/*.bash $(contrib-subdirs:%=contrib/%/*.bash)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, you are right. Please add an entry for the pl file. Something like
| # FIXME: I belive the ble.sh author encouraged me to put the perl-re-server2 in | |
| # it's own file, but for it to get correctly installed it would have to get | |
| # mentioned hereish | |
| contrib-srcfiles := $(wildcard contrib/*.bash $(contrib-subdirs:%=contrib/%/*.bash)) | |
| contrib-srcfiles := $(wildcard contrib/*.bash $(contrib-subdirs:%=contrib/%/*.bash)) \ | |
| contrib/config/github302-perlre-server2.pl |
| # FIXME: These keymap functions need to be renamed. They are private so in | ||
| # theory should get the full treatment from style guide hmm I dunno maybe | ||
| # ask author |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current naming is a good one.
I realized just now that I haven't mentioned the widget names in the style guide, but basically, all the widgets that are supposed to be bound by ble-bind are public in the sense that we would like to allow the users to change the key the widget is bound to. Alos, I prefer a shorter name for the widgets instead of something like contrib/github302/vim-wordmotion-ish/blahblah.
Specifically we incoporate the changes to keymap.vi.sh from upstream 11cf118af260e19ff7e7c1565b67d3f86cf99098
This is a slight elaboration on the earlier perlre-server st all group info is returned. This is the version my clone of
chaoren/vim-wordmotion uses (I'm hoping to submit that to contrib somewhere soon).
This is some output when restart happens (not really a problem and maybe desirable):