Re: Re: Re: [PATCH 0/4] Add import builtin

[Matheus Afonso Martins Moreira]

>> A native way to source libraries. Built into bash, available to all users.
> That's the source builtin.

It looks for scripts in directories meant for executables.
It prefers files with the executable bit set.
It's a native way to load bash scripts for sure.
But it's not a native way to load libraries.

> It's just your expectation of the ergonomics but not ours.

What are your expectations?

Do you really think it's reasonable to suggest that
users implement their own library importing function?

The fact such a function can be written in bash
is evidence of how powerful bash is. It doesn't
really follow though that everything that can be
written in bash should be done in bash.

> He submitted patch seems unneededly large.

The diffstat shows 9 files changed,
203 insertions, 92 deletions.

Most of those are very simple
refactorings of existing code.

> incomplete addition of the long option of builtins

What do you mean? What is missing?

> an incomplete support for the XDG base directories, which should
> actually be discussed separately.

XDG does not specify the ~/.local and ~/.local/lib directories.
Only the XDG Data Directory defaults to ~/.local/share if unset.
That implies a ~/.local directory tree that mirrors that of /usr.
Seemed simple enough so I just used that path. However, it is
not actually specified anywhere and there are no variables for
those directories. So I don't think there's anything that needs
to be supported here.

Also, it appears the matter of XDG variables
has already been decided:

https://savannah.gnu.org/support/?108134
https://savannah.gnu.org/patch/?10431

>  It does seem your personal hangup not giving a filename extension to
> the script files, which ended up with mixed script files to be sourced
> and executable files.

Whether to use file extensions or not is the user's choice.
Bash should work correctly regardless of what the file name is.

Besides, using file extensions does not actually prevent
false positives. Scripts often end in .sh and .bash too.
I have named them that way before.

File extensions do not enforce any hard separation
between executables and libraries. Directories do that.

> If you think that is the problem, your suggestion doesn't actually
> solve the problem of module managers.

I didn't think it was a problem at first. I just thought it was unnecessary.
Bash already knows how to do it. We should make it do it for us.

Now I do see it as a problem to be solved. Bash has a restricted shell mode
which, among many other things, prohibits slashes in file names passed to
the source builtin. If things are as you claim and the module managers really
do resolve the paths themselves in order to pass an absolute path to source,
that means they are all incompatible with bash's restricted shells.
The proposed native implementation of the module loader is not.

> Module managers will have to implement it by themselves to satisfy
> their needs (which is obviously different from your ergonomics)
> even if your feature is added to Bash.

That's true. When I mentioned ergonomics, I wasn't really talking about
the module managers though. I was talking from the perspective of a
user who just launched a terminal emulator, got a bash prompt
and now wants to source some custom functions.

>  Is this your rebuttal to Lawrence's "The sourced files that can be
>  written with your feature available are exactly the same as the ones
>  that can be written right now. Much like the periodic requests for
>  XDG-organized startup files.".

No. It was not actually a rebuttal at all. It was my response
to the exact text I quoted in the message. Specifically, this text:

> Much like the periodic requests for XDG-organized startup files

I'd really appreciate it if my contribution wasn't regarded
as just the latest iteration in the infinite loop of annoying
XDG Base Directory implementation requests from users.

> If you avoid discussion by showing the relation with the authority,
> it seems a proof of lacking a straightforward counter argument

I avoided discussion because I started to feel like
I wasn't being taken seriously. I was discussing
the matter with you in good faith before that.

> Chet mentioned the additional variable for the locations to search,
> but not a new builtin or a flag.

That's true. I did that of my own initiative.
After some discussion in the help-bash list,
I began to think that a builtin was justified.
Then after discussing it with you I changed
my mind and rewrote the patch so that it's an
option for the source builtin instead of an
entirely new builtin. You made excellent points
and you convinced me that a new builtin was
the wrong approach.

I understand that it's not exactly what was discussed.
However, I did have a reason for doing it that way.
I think the logic behind the way source uses PATH
is somewhat confusing and adding a new variable
on top of all that would only confuse it further.
For that reason, I added an option instead.
If you pass the option, it uses the library variable.
There are no nested conditionals to think about.

I think that is easier for users to understand.
It certainly is easier for me to understand.

> However, as far as I understand, no one has requested you
> to work on this. You've voluntarily appeared on the help-bash and
> bug-bash lists and submitted patches.

Was this inappropriate?

> It's unfair to request 100% merge

I did not request that. I asked for consideration.
If it's rejected, I will accept that. I'm sure the maintainer
will have good reasons for rejecting it. I made it a point
to organize the commits so that valuable contributions
could still be cherry picked even in the event of rejection.

> Do we need to merge every patch unconditionally after discussions? I
> don't think so.

That's not what I meant. I'm not demanding acceptance.
I just don't want to be seen as a guy who just shows up
out of nowhere and demands that others do free work
for him on the features he personally thinks are important.

I cloned the repository and made bash do what I wanted.
Then I shared the results of that effort with everyone here.
What happens after that is up to the community.

> The code change seems too large considering the amount of the feature
> change, but I haven't yet checked the code. I'll later check them.

Perhaps the number of patches gave everyone that impression.
Many are just rather small incremental changes that lay out the
groundwork for the feature. Changes such as extracting some
logic into a parameterized helper function and converting
hard coded strings into function parameters.

Patches 3 and 6-9 implement the feature proper.
The others could be cherry picked independently.
  
> You should be prepared to receive negative comments in the review
> process. The reviews are not always 100% positive. This is normal.

I can handle criticism of my code. It wasn't really my intention to be a
source of irritation to the community though. That made me think
I should probably just leave instead of continuing these arguments.

    Matheus