Reload file

[insunaa]

Adds a menu option to reload the current file. Normally when a file is changed from outside of juCi++, it first has to be closed completely and then re-opened in order to work with these changes.

In case of a race while using this PR (a file was changed externally and juCi++ has unsaved changes), upon clicking "Reload File" in the menu, the function will always be executed, but the user is asked if they want to save the current file before it's opened. There's no merging and no backups, either the file on the drive is overwritten or the changes within the editor are overwritten.

To fix this, it would make sense to add an extra warning beforehand, or, instead of an overwriting save, open the "Save File As" prompt, to give the user the option to save his current changes to a separate file and manually merge them afterwards.

I don't know how to do that, so it's not in this pull request.

This pull request addresses this issue I opened earlier: https://github.com/cppit/jucipp/issues/285

I'm going to try to find out how to solve the race-condition, but I'd like your input on that.

[eidheim]

Thank you, this is a good idea in case one is working on multiple branches. I think a yes/no dialog should be enough though on the reload action. Also the directories has to be updated (Directory::get().update()) on this action (edit: I think this will happen automatically), and the source_diff's configure has to be run I think (without having looked at the source code). There should maybe be a update method in source_diff if there is no already, and this should be called instead of source_diff's configure I guess. Edit: well this should be updated automatically as well! Sorry, it's been a long day:)

[insunaa]

What do they do? Because as it is right now it really just does reload the current file from the directory and treats it as a newly opened file (so it's parsed again)

[eidheim]

See my updated comment:)

By the way, no need to close and reopen the tab, you can just clear the buffer and insert the contents of the new file. One should also try to go the the last cursor position if it's still there, if not somewhere nearby, and do a scroll to that position.

[insunaa]

I went with the path of least resistance here. I think that clearing/replacing the buffer with the contents would require new safety checks by hand. I'll look into it, but I can make no promises for success :)

Edit: I can't really figure out how to properly reopen the file another way. I tried by dissecting the

Notebook::get().open(path);

method and building a proper reload function, but I just don't understand it well enough to do it properly.

[insunaa]

I've experimented quite a bit and I can't get it to work properly. Whenever I try to create a new textiter in the new buffer to place the cursor, the application instead marks the text from offset 0 to the cursor offset and if I manually place the cursor outside of the previously marked text, the whole application crashes with segfault and error code 139

I'm definitely doing something wrong, but I can't figure out what.

Same goes if I only replace the current current buffer with the new buffer and try to save (segfault, return code 139)

The "this file was changed outside of juCi++" warning also doesn't go away.

void Notebook::reload(const boost::filesystem::path &file_path, size_t notebook_index) {
  if(boost::filesystem::exists(file_path)) {
    std::ifstream can_read(file_path.string());
    if(!can_read) {
      Terminal::get().print("Error: could not open "+file_path.string()+"\n", true);
      return;
    }
    can_read.close();
  }

  auto last_view=get_current_view();
  Source::View* new_view;

  auto language=Source::guess_language(file_path);
  if(language && (language->get_id()=="chdr" || language->get_id()=="cpphdr" || language->get_id()=="c" || language->get_id()=="cpp" || language->get_id()=="objc"))
    new_view = new Source::ClangView(file_path, language);
  else
    new_view = new Source::GenericView(file_path, language);

  auto insert = last_view->get_buffer()->get_insert();
  int offset = insert->get_iter().get_offset();
  auto name = insert->get_name();
  auto old_position = new_view->get_iter_at_line_end(0);
  old_position.set_offset(offset);

  last_view->set_buffer(new_view->get_buffer());
  last_view->get_buffer()->create_mark(name, old_position);
  last_view->get_buffer()->set_modified(false);
}

this is what it currently looks like. It's pretty wrong, obviously, but I don't know how to do it right.

The reason for

  auto old_position = new_view->get_iter_at_line_end(0);

Is that I somehow can't create a new TextIter for the new buffer and Gtk really doesn't like it if I don't use a TextIter or mark from the target document (for good reason)

[eidheim]

I don't have time to look at this right now, but you can use this new function I added some weeks back: https://github.com/cppit/jucipp/blob/master/src/source.h#L51. It handles the gtk specifics related to placing the cursor somewhere nearby its previous position after buffer update.

[insunaa]

Ahh, I see now why it's crashing >_>

I'm letting an object run out of scope but I'm still referencing a deallocated pointer out of scope...

Can't believe I didn't spot that yesterday... The buffer is a Glib::RefPtr<Gtk::TextBuffer>, but the parent of this buffer is running out of scope. Any idea how I can let the parent run out of scope but preserve the buffer itself?

Edit: I also don't know how many things I also have to change after replacing the buffer.

Right now it looks like this:

void Notebook::reload(const boost::filesystem::path &file_path, size_t notebook_index) {
  if(boost::filesystem::exists(file_path)) {
    std::ifstream can_read(file_path.string());
    if(!can_read) {
      Terminal::get().print("Error: could not open "+file_path.string()+"\n", true);
      return;
    }
    can_read.close();
  }

  auto last_view=get_current_view();
  int offset = last_view->get_buffer()->get_insert()->get_iter().get_offset();
  int line = last_view->get_buffer()->get_insert()->get_iter().get_line();
  Source::View* new_view;

  auto language=Source::guess_language(file_path);
  if(language && (language->get_id()=="chdr" || language->get_id()=="cpphdr" || language->get_id()=="c" || language->get_id()=="cpp" || language->get_id()=="objc"))
    new_view = new Source::ClangView(file_path, language);
  else
    new_view = new Source::GenericView(file_path, language);

  auto textbuffer = new_view->get_buffer();

  Gtk::TextBuffer *a = textbuffer.release();
  Glib::RefPtr<Gtk::TextBuffer> b = Glib::RefPtr<Gtk::TextBuffer>(a);
  last_view->set_buffer(b);

  last_view->place_cursor_at_line_offset(line, offset);
  last_view->get_buffer()->set_modified(true);
}

It's still segfaulting, tho I don't think it's because the buffer runs out of scope (I'm not sure what RefPtr does, but I think the pointer should be kept alive if I'm handing it over?) but it's segfaulting because of some other misalignment between view and buffer.

Edit 2:

void Notebook::reload(const boost::filesystem::path &file_path, size_t notebook_index) {
  if(boost::filesystem::exists(file_path)) {
    std::ifstream can_read(file_path.string());
    if(!can_read) {
      Terminal::get().print("Error: could not open "+file_path.string()+"\n", true);
      return;
    }
    can_read.close();
  }

  auto last_view=get_current_view();
  int offset = last_view->get_buffer()->get_insert()->get_iter().get_offset();
  int line = last_view->get_buffer()->get_insert()->get_iter().get_line();
  Source::View* new_view;

  auto language=Source::guess_language(file_path);
  if(language && (language->get_id()=="chdr" || language->get_id()=="cpphdr" || language->get_id()=="c" || language->get_id()=="cpp" || language->get_id()=="objc"))
    new_view = new Source::ClangView(file_path, language);
  else
    new_view = new Source::GenericView(file_path, language);

  last_view->set_source_buffer(new_view->get_source_buffer());

  last_view->place_cursor_at_line_offset(line, offset);
  last_view->get_buffer()->set_modified(true);
}

This seems to work better, it replaces the buffer nicely, but after around 5 seconds it segfaults again.

Edit 3: It's a very very weird bug, because at the moment it doesn't segfault again for some reason, even tho I only made and rolled back a few changes... to this exact state...

I'm so confused, pls help.

Edit 4: And after a complete recompile it segfaults again... I have no idea what's going on and I have no idea how gdb works...

Edit 5: The debugger says it's nullref-ing somewhere. I assume that the view in it's destructor calls something like buffer->reset() and sets the buffer to a null reference, even tho the buffer was already handed over to another parent. I've also tried releasing the buffer and giving the new view a copy of the buffer, but that didn't work either.

This is what I'm getting as console output:

(juci:10797): Gtk-CRITICAL **: gtk_text_buffer_delete_mark: assertion '!gtk_text_mark_get_deleted (mark)' failed

(juci:10797): Gtk-CRITICAL **: gtk_text_buffer_delete_mark: assertion '!gtk_text_mark_get_deleted (mark)' failed sh: line 1: 10797 Segmentation fault (core dumped) /home/username/git/d3rrial_jucipp/jucipp/build/src/juci /home/username/git/d3rrial_jucipp/jucipp/build/src/juci returned: 139

Edit 6: If I replace both buffer and source_buffer, it takes a lot longer, but a segfault will still inevitably occur...

[eidheim]

There should be no need to create a new Source::View since the file should have the same extension. Deleting a view is somewhat complex since it has to be done in the background due to there are no way of stopping libclang while it's processing. I think this will solve your issues.

[insunaa]

OK, added a new commit that does what you described and works without any crashes. There are the following issues, which I can probably only resolve if I work on the newest build (probably) which I am currently not (because of difficulties with forking on github):

• The "File was altered outside of juCi++" warning remains after having reloaded the file. I'll need it to reset the last write time, so that this warning goes away.
• Reloading the file can't be undone with Ctrl+Z. I've done that on purpose here: last_view->get_source_buffer()->begin_not_undoable_action();, because last_view->get_buffer()->erase(last_view->get_buffer()->begin(), last_view->get_buffer()->end()); is seperate on the undo stack from loading the buffer, which means that hitting undo would first make the editor completely empty and only after a second undo would it restore to the original state. I don't know how to solve that.

Edit: I've chosen to work on the old TextBuffer instead of creating a new one, because that would lose all kinds of important settings, which I'd have to reapply manually.

[eidheim]

Thank you! I did some changes, but it's merged now:)