Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[WIP] share: Introduce the sh directory, mktemp implementation #1430

Closed
wants to merge 1 commit into from

Conversation

lenormf
Copy link
Contributor

@lenormf lenormf commented Jun 9, 2017

This commit installs an sh directory in the share directory, in which
implementations of popular tools that might not be available on a user's
system sit.

In order for a shell scope to use those implementations if the regular
binaries are unavailable, the PATH variable has to be modified as follows,
at the beginning of the scope:

%sh{
    export PATH="${PATH}:${kak_runtime}/sh"

    <code>
}

@lenormf lenormf force-pushed the posix-commands branch 3 times, most recently from e334f54 to 9d993ca Compare June 9, 2017 13:06
rc/base/lint.kak Outdated
@@ -8,6 +8,8 @@ decl -hidden range-specs lint_errors

def lint -docstring 'Parse the current buffer with a linter' %{
%sh{
export PATH="${PATH}:${kak_runtime}/sh"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a '# mktemp fallback' comment should follow, to make it explicit which fallback we try to import

@lenormf
Copy link
Contributor Author

lenormf commented Jun 10, 2017

I added a comment hinting that we modify the path to have access to the mktemp custom implementation.

@lenormf
Copy link
Contributor Author

lenormf commented Jun 14, 2017

As it turned out after merging #1429, even such a simple utility uses different flags depending on the environment (GNU or mac basically). I think we should not extend the PATH and use the implementation as a fallback in those cases (i.e. mktemp), and have a wrapper/reimplementation named differently instead (kak_mktemp).

@mawww
Copy link
Owner

mawww commented Jun 16, 2017

Yeah, I am still on the fence about this change (I know I suggested it 😛), I tend to think we should not start adding a set of additional commands bundled with Kakoune, but we are hitting these portability problems from time to time.

Kakoune should not try to fix posix problems itself, we try to stay as posix as possible to maximize portability, but if we dont have an acceptable tool provided by posix, I am not sure we should be rolling our own instead of just depending on the lowest common denominator that is usually available.

This needs more discussions and thoughts...

@lenormf
Copy link
Contributor Author

lenormf commented Jun 16, 2017

Note that the original idea was to provide a minimal fallback implementation in case the tools were not available on the system. I'm OK with not trying to abstract already existing APIs (e.g. GNU vs BSD mktemp).

I'll rework the mktemp implementation to make it only accept a -d flag, and require a template as argument (it seems to be what's working on both linux and mac).

This commit installs an `sh` directory in the `share` directory, in which
implementations of popular tools that might not be available on a user's
system sit.

In order for a shell scope to use those implementations if the regular
binaries are unavailable, the `PATH` variable has to be modified as follows,
at the beginning of the scope:

```
%sh{
    export PATH="${PATH}:${kak_runtime}/sh"

    <code>
}
```
@lenormf
Copy link
Contributor Author

lenormf commented Jun 16, 2017

Rebased on f9c4823, simplify the mktemp alternative implementation to be seamlessly usable with the current code.

@lenormf lenormf changed the title [WIP] share: Introduce the sh directory, mktemp implementation share: Introduce the sh directory, mktemp implementation Jun 16, 2017
@mawww
Copy link
Owner

mawww commented Jun 17, 2017

I am wondering on which platform dont we have mktemp available ?

@lenormf
Copy link
Contributor Author

lenormf commented Jun 17, 2017

Good point, mktemp is also provided by busybox, which I use as a reference for availability of a command.

So really the only script we could use now is readlink, right ?

@lenormf lenormf changed the title share: Introduce the sh directory, mktemp implementation [WIP] share: Introduce the sh directory, mktemp implementation Jun 23, 2017
@lenormf lenormf closed this Oct 4, 2017
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants