buffer to_buffer(string)

[Veracity]

This has been missing since forever.

Instead of:

buffer buf;
buf.append("Initial content");

It's be nice to have:

buffer buf = to_buffer("Initial content");

or even, with auto-coercion:

buffer buf = "Initial content";

I have a PR which adds that function - and auto-coercion - to the ASH RuntimeLibrary. Once it is merged, scripts that have their own version of this, having noticed the glaring omission, will get a compile error since they are overriding a function in the RuntimeLibrary.

If I recall, we had a similar situation when gausie added to_path(string).
What did we do?

What if we made that a warning, rather than an error?

 

[Ryo_Sangnoir]

What if we made that a warning, rather than an error?

I think that's a great idea! The other thing to decide is whether it should call the one you've defined or the inbuilt one: I think it should call the one you defined. This will let scripts keep working as they expect.

Path was a bit different as I recall: there, the issue was that lots of scripts were doing 'my_path() == "Trendy"' and we wanted that to keep working, even though my_path() now returned a string.

 

[MCroft]

It would make things more flexible if script authors could override built_in functions, but I'm wondering what the implications are of letting a script override a built in, especially if the script gets included in another script. What if they had differing definitions? While it won't be the case with to_buffer(), what if they have different result types? (I think that still has to be a warning).

I do see " to_buffer" and "(to_buffer" in a number of the scripts I use, so there's definitely going to be an impact.

Here's a quick look at my list, de-duped and trimmed of the actual content (for brevity) and .svn and commented out code:
Michaels-MBP:kolmafia mcroft$ grep -r " to_buffer" *

git/midgleyc-Bastille-Release/scripts/bastille.ash
git/Loathing-Associates-Scripting-Society-TourGuide-Release/relay/relay_TourGuide.ash
svn/Ezandora-Guide-branches-Release/relay/relay_Guide.ash
svn/Ezandora-Helix-Fossil-branches-Release/scripts/Helix Fossil/Pocket Familiars.ash
svn/Ezandora-Genie-branches-Release/scripts/genie.ash
svn/Ezandora-Detective-Solver-branches-Release/scripts/Detective Solver.ash
svn/Ezandora-Briefcase-branches-Release/scripts/Briefcase.ash

Some of these I haven't run in ages, or looked for a GIT version.

 

[MCroft]

Looking into the Bastille code, which has (and uses) their own to_buffer, I also see a defined function buffer copyBuffer(buffer), which seems like it's the equivalent, except it takes a buffer instead of a string.

It's not used in Bastille, but is in all the same Ezandora-derived scripts and I didn't dig to see if it was in use in everything.

Do we currently support buffer buf = otherbuf ?

Are there other Buffer related create or manipulate commands we might consider? For example, we have buffer replace, but string replace_all and replace_first. We can build those out of to_buffer(replace_all()), though, so maybe not?

I don't do enough scripting to have an opinion on what the pain points of converting between string and buffer are.

 

[Veracity]

Assigning a buffer to a variable of type buffer gives you two variables which point to the sane buffer; we never copy mutable data structures like records, maps, or buffers when we assign them.

 

[MCroft]

This is the function I'm seeing. Is this a function we should adopt? It sounds like the answer is no.

buffer copyBuffer(buffer buf) { buffer result; result.append(buf); return result; }

 

[Veracity]

That looks like buffer to_buffer(buffer);

 

[Veracity]

OK, I'm going to make it a warning, rather than an error.
That should eliminate complaints from users that their scripts broke.
Instead, we'll have complaints that their scripts now print out a warning when you execute them for the first time.