Refactor buffer creation and reloading to be more explicit

Reloading used to be implicit in the buffer creation function,
which is not always nice, as code that explicitely wanted to
reload a buffer could not say so.

src/buffer_utils.cc

@@ -2,6 +2,7 @@
 
 #include "buffer_manager.hh"
 #include "event_manager.hh"
+#include "file.hh"
 
 #include <unistd.h>
 #include <sys/select.h>
@@ -46,50 +47,83 @@ ByteCount get_byte_to_column(const Buffer& buffer, CharCount tabstop, CharCoord
     return (int)(it - line.begin());
 }
 
-Buffer* create_buffer_from_data(StringView data, StringView name,
+struct BufferData
-                                Buffer::Flags flags, timespec fs_timestamp)
 {
-    bool bom = false, crlf = false;
-
-    const char* pos = data.begin();
-    if (data.length() >= 3 and
-       data[0_byte] == '\xEF' and data[1_byte] == '\xBB' and data[2_byte] == '\xBF')
-    {
-        bom = true;
-        pos = data.begin() + 3;
-    }
-
     BufferLines lines;
-    while (pos < data.end())
+    bool bom = false;
+    bool crlf = false;
+
+    BufferData(StringView data)
     {
-        const char* line_end = pos;
+        const char* pos = data.begin();
-        while (line_end < data.end() and *line_end != '\r' and *line_end != '\n')
+        if (data.length() >= 3 and
-             ++line_end;
+           data[0_byte] == '\xEF' and data[1_byte] == '\xBB' and data[2_byte] == '\xBF')
-
-        lines.emplace_back(StringData::create({pos, line_end}, '\n'));
-
-        if (line_end+1 != data.end() and *line_end == '\r' and *(line_end+1) == '\n')
         {
-            crlf = true;
+            bom = true;
-            pos = line_end + 2;
+            pos = data.begin() + 3;
+        }
+
+        while (pos < data.end())
+        {
+            const char* line_end = pos;
+            while (line_end < data.end() and *line_end != '\r' and *line_end != '\n')
+                 ++line_end;
+
+            lines.emplace_back(StringData::create({pos, line_end}, '\n'));
+
+            if (line_end+1 != data.end() and *line_end == '\r' and *(line_end+1) == '\n')
+            {
+                crlf = true;
+                pos = line_end + 2;
+            }
+            else
+                pos = line_end + 1;
         }
-        else
-            pos = line_end + 1;
     }
 
-    Buffer* buffer = BufferManager::instance().get_buffer_ifp(name);
+    void apply_options(Buffer& buffer) const
-    if (buffer)
+    {
-        buffer->reload(std::move(lines), fs_timestamp);
+        OptionManager& options = buffer.options();
-    else
+        options.get_local_option("eolformat").set<String>(crlf ? "crlf" : "lf");
-        buffer = new Buffer{name.str(), flags, std::move(lines), fs_timestamp};
+        options.get_local_option("BOM").set<String>(bom ? "utf-8" : "no");
+    }
+};
 
-    OptionManager& options = buffer->options();
+Buffer* create_file_buffer(StringView filename)
-    options.get_local_option("eolformat").set<String>(crlf ? "crlf" : "lf");
+{
-    options.get_local_option("BOM").set<String>(bom ? "utf-8" : "no");
+    if (MappedFile file_data{filename})
+        return create_buffer({ file_data.data, (int)file_data.st.st_size }, filename,
+                             Buffer::Flags::File, file_data.st.st_mtim);
+    return nullptr;
+}
 
+bool reload_file_buffer(Buffer& buffer)
+{
+    kak_assert(buffer.flags() & Buffer::Flags::File);
+    if (MappedFile file_data{buffer.name()})
+    {
+        reload_buffer(buffer,  { file_data.data, (int)file_data.st.st_size }, file_data.st.st_mtim);
+        return true;
+    }
+    return false;
+}
+
+Buffer* create_buffer(StringView data, StringView name, Buffer::Flags flags,
+                      timespec fs_timestamp)
+{
+    BufferData buf_data(data);
+    Buffer* buffer = new Buffer{name.str(), flags, std::move(buf_data.lines), fs_timestamp};
+    buf_data.apply_options(*buffer);
     return buffer;
 }
 
+void reload_buffer(Buffer& buffer, StringView data, timespec fs_timestamp)
+{
+    BufferData buf_data(data);
+    buffer.reload(std::move(buf_data.lines), fs_timestamp);
+    buf_data.apply_options(buffer);
+}
+
 Buffer* create_fifo_buffer(String name, int fd, bool scroll)
 {
     static ValueId s_fifo_watcher_id = ValueId::get_free_id();
@@ -185,7 +219,7 @@ void write_to_debug_buffer(StringView str)
     else
     {
         String line = str + ((str.empty() or str.back() != '\n') ? "\n" : "");
-        create_buffer_from_data(line, debug_buffer_name, Buffer::Flags::NoUndo);
+        create_buffer(line, debug_buffer_name, Buffer::Flags::NoUndo, InvalidTime);
     }
 }
 

src/buffer_utils.hh

@@ -30,11 +30,13 @@ CharCount get_column(const Buffer& buffer,
 ByteCount get_byte_to_column(const Buffer& buffer, CharCount tabstop,
                              CharCoord coord);
 
-Buffer* create_fifo_buffer(String name, int fd, bool scroll = false);
+Buffer* create_buffer(StringView data, StringView name,
+                      Buffer::Flags flags, timespec fs_timestamp);
+void reload_buffer(Buffer& buffer, StringView data, timespec fs_timestamp);
 
-Buffer* create_buffer_from_data(StringView data, StringView name,
+Buffer* create_fifo_buffer(String name, int fd, bool scroll = false);
-                                Buffer::Flags flags,
+Buffer* create_file_buffer(StringView filename);
-                                timespec fs_timestamp = InvalidTime);
+bool reload_file_buffer(Buffer& buffer);
 
 void write_to_debug_buffer(StringView str);
 

src/client.cc

@@ -185,14 +185,10 @@ void Client::force_redraw()
 
 void Client::reload_buffer()
 {
-    auto& buffer = context().buffer();
+    Buffer& buffer = context().buffer();
-    kak_assert(buffer.flags() & Buffer::Flags::File);
+    if (reload_file_buffer(buffer))
-    if (Buffer* buf = create_buffer_from_file(buffer.name()))
-    {
-        kak_assert(buf == &buffer);
         context().print_status({ format("'{}' reloaded", buffer.display_name()),
                                  get_face("Information") });
-    }
     else
         context().print_status({ format("could not reload '{}'", buffer.display_name()),
                                  get_face("Error") });

src/commands.cc

@@ -133,11 +133,12 @@ void edit(const ParametersParser& parser, Context& context)
                                                : context.buffer().name();
     auto& buffer_manager = BufferManager::instance();
 
-    Buffer* buffer = nullptr;
+    Buffer* buffer = buffer_manager.get_buffer_ifp(name);
     Buffer* oldbuf = &context.buffer();
-    if (not force_reload)
+    // TODO fifo reload
-        buffer = buffer_manager.get_buffer_ifp(name);
+    if (force_reload and buffer and buffer->flags() & Buffer::Flags::File)
-    if (not buffer)
+        reload_file_buffer(*buffer);
+    else
     {
         if (parser.get_switch("scratch"))
         {
@@ -151,9 +152,9 @@ void edit(const ParametersParser& parser, Context& context)
         }
         else if (auto fifo = parser.get_switch("fifo"))
             buffer = open_fifo(name, *fifo, (bool)parser.get_switch("scroll"));
-        else
+        else if (not buffer)
         {
-            buffer = create_buffer_from_file(name);
+            buffer = create_file_buffer(name);
             if (not buffer)
             {
                 if (parser.get_switch("existing"))

src/file.cc

@@ -2,15 +2,11 @@
 
 #include "assert.hh"
 #include "buffer.hh"
-#include "buffer_manager.hh"
-#include "buffer_utils.hh"
 #include "unicode.hh"
 #include "regex.hh"
 #include "string.hh"
 
 #include <errno.h>
-#include <sys/types.h>
-#include <sys/stat.h>
 #include <sys/mman.h>
 #include <fcntl.h>
 #include <unistd.h>
@@ -161,33 +157,33 @@ String read_file(StringView filename, bool text)
     return read_fd(fd, text);
 }
 
-Buffer* create_buffer_from_file(StringView filename)
+MappedFile::MappedFile(StringView filename)
 {
     String real_filename = real_path(parse_filename(filename));
 
-    int fd = open(real_filename.c_str(), O_RDONLY | O_NONBLOCK);
+    fd = open(real_filename.c_str(), O_RDONLY | O_NONBLOCK);
     if (fd == -1)
     {
         if (errno == ENOENT)
-            return nullptr;
+            return;
 
         throw file_access_error(real_filename, strerror(errno));
     }
-    auto close_fd = on_scope_end([&]{ close(fd); });
 
-    struct stat st;
     fstat(fd, &st);
     if (S_ISDIR(st.st_mode))
         throw file_access_error(real_filename, "is a directory");
-    if (S_ISFIFO(st.st_mode)) // Do not try to read fifos, use them as write only
-        return create_buffer_from_data({}, real_filename,
-                                       Buffer::Flags::File, st.st_mtim);
 
-    const char* data = (const char*)mmap(nullptr, st.st_size, PROT_READ, MAP_PRIVATE, fd, 0);
+    data = (const char*)mmap(nullptr, st.st_size, PROT_READ, MAP_PRIVATE, fd, 0);
-    auto unmap = on_scope_end([&]{ munmap((void*)data, st.st_size); });
+}
 
-    return create_buffer_from_data({data, data + st.st_size}, real_filename,
+MappedFile::~MappedFile()
-                                   Buffer::Flags::File, st.st_mtim);
+{
+    if (fd != -1)
+    {
+        munmap((void*)data, st.st_size);
+        close(fd);
+    }
 }
 
 static void write(int fd, StringView data)

src/file.hh

@@ -6,6 +6,9 @@
 #include "exception.hh"
 #include "regex.hh"
 
+#include <sys/types.h>
+#include <sys/stat.h>
+
 namespace Kakoune
 {
 
@@ -40,7 +43,17 @@ String get_kak_binary_path();
 String read_fd(int fd, bool text = false);
 String read_file(StringView filename, bool text = false);
 
-Buffer* create_buffer_from_file(StringView filename);
+struct MappedFile
+{
+    int fd;
+    const char* data = nullptr;
+    struct stat st {};
+
+    explicit operator bool() const { return fd != -1; }
+
+    MappedFile(StringView filename);
+    ~MappedFile();
+};
 
 void write_buffer_to_file(Buffer& buffer, StringView filename);
 void write_buffer_to_fd(Buffer& buffer, int fd);

src/main.cc

@@ -500,7 +500,7 @@ int run_server(StringView session, StringView init_command,
         {
             try
             {
-                if (not create_buffer_from_file(file))
+                if (create_file_buffer(file) == nullptr)
                     new Buffer(file.str(), Buffer::Flags::New | Buffer::Flags::File);
             }
             catch (Kakoune::runtime_error& error)
@@ -593,7 +593,7 @@ int run_filter(StringView keystr, ConstArrayView<StringView> files, bool quiet)
 
         for (auto& file : files)
         {
-            Buffer* buffer = create_buffer_from_file(file);
+            Buffer* buffer = create_file_buffer(file);
             write_buffer_to_file(*buffer, file + ".kak-bak");
             apply_keys_to_buffer(*buffer);
             write_buffer_to_file(*buffer, file);
@@ -601,8 +601,8 @@ int run_filter(StringView keystr, ConstArrayView<StringView> files, bool quiet)
         }
         if (not isatty(0))
         {
-            Buffer* buffer = create_buffer_from_data(read_fd(0), "*stdin*",
+            Buffer* buffer = create_buffer(read_fd(0), "*stdin*",
-                                                     Buffer::Flags::None);
+                                           Buffer::Flags::None, InvalidTime);
             apply_keys_to_buffer(*buffer);
             write_buffer_to_fd(*buffer, 1);
             buffer_manager.delete_buffer(*buffer);

src/normal.cc

@@ -191,7 +191,10 @@ void goto_commands(Context& context, NormalParams params)
                 if (path.empty())
                     throw runtime_error(format("unable to find file '{}'", filename));
 
-                Buffer* buffer = create_buffer_from_file(path);
+                Buffer* buffer = BufferManager::instance().get_buffer_ifp(path);
+                if (not buffer)
+                    buffer = create_file_buffer(path);
+
                 if (buffer == nullptr)
                     throw runtime_error(format("unable to open file '{}'", path));