New issue

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

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

Already on GitHub? Sign in to your account

Fix change detection when adding files to a sketch #4849

Closed
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@GKFX
Contributor

GKFX commented Jan 23, 2017

Change Detector now differentiates between a merge conflict (same file changed; will load the external changes if accepted by user; discards editor content) or a non-conflicting change (will load the external changes if accepted by user; nothing is lost). This is done per file so that if one tab does conflict and the next doesn't, the non-conflicting tab is not discarded for no reason.
Fixes #4713.

Make it safe to add tab files to an open sketch.
Change Detector now differentiates between a merge conflict (same file
changed; will load the external changes if accepted by user; discards
editor content) or a non-conflicting change (will load the external
changes if accepted by user; nothing is lost).
@benfry

This comment has been minimized.

Show comment
Hide comment
@benfry

benfry Jan 29, 2017

Member

@JakubValtar Can you review this? You were last in the change detection code…

Member

benfry commented Jan 29, 2017

@JakubValtar Can you review this? You were last in the change detection code…

@JakubValtar

This comment has been minimized.

Show comment
Hide comment
@JakubValtar

JakubValtar Jan 29, 2017

Contributor

@benfry Yes, sure thing.

Contributor

JakubValtar commented Jan 29, 2017

@benfry Yes, sure thing.

@JakubValtar

This comment has been minimized.

Show comment
Hide comment
@JakubValtar

JakubValtar Jan 29, 2017

Contributor

@GKFX I like your idea and it works as expected, I just think the dialogs are way too complicated and as a user I'm really confused. This makes my brain melt :)

image

@benfry How do you think this should work? It's more of a design problem.

Current approach is to either

  • scrap all changes in PDE and reload the sketch from the disk,
  • or keep stuff in PDE and just mark appropriate tabs as modified.

Problem is when you have modified tabs and you change one file externally, you either have to scrap changes in all tabs and reload the sketch, or ignore the external changes.

The approach proposed in this PR is to detect tabs which were modified by external program (timestamp changed), and either

  • save contents of all tabs modified only in PDE and reload the sketch (this is because there is no code to reload only some tabs so we have to save what we want to keep and reload the whole sketch),
  • or keep stuff in PDE and just mark appropriate tabs as modified.

The idea is good because if you modify something externally, you probably want to reload just that and keep other tabs intact. The problem is that we have to save all other tabs in order to reload only one file, and it's really hard to explain what is going on.

I think to address the problem we should rather write code to reload separate tabs and show one dialog per tab (like other editors):

  • modified: File "file2.pde" has been modified by another program, do you want to reload it?
  • modified both externally and in PDE: File "file3.pde" has been modified by another program, do you want to reload it and lose changes from Processing?
  • deleted: File "file1.pde" was deleted, keep it in editor?
  • added: Add file to sketch automatically. If it's in the folder, it's part of the sketch and will appear anyway when you open the sketch next time.
Contributor

JakubValtar commented Jan 29, 2017

@GKFX I like your idea and it works as expected, I just think the dialogs are way too complicated and as a user I'm really confused. This makes my brain melt :)

image

@benfry How do you think this should work? It's more of a design problem.

Current approach is to either

  • scrap all changes in PDE and reload the sketch from the disk,
  • or keep stuff in PDE and just mark appropriate tabs as modified.

Problem is when you have modified tabs and you change one file externally, you either have to scrap changes in all tabs and reload the sketch, or ignore the external changes.

The approach proposed in this PR is to detect tabs which were modified by external program (timestamp changed), and either

  • save contents of all tabs modified only in PDE and reload the sketch (this is because there is no code to reload only some tabs so we have to save what we want to keep and reload the whole sketch),
  • or keep stuff in PDE and just mark appropriate tabs as modified.

The idea is good because if you modify something externally, you probably want to reload just that and keep other tabs intact. The problem is that we have to save all other tabs in order to reload only one file, and it's really hard to explain what is going on.

I think to address the problem we should rather write code to reload separate tabs and show one dialog per tab (like other editors):

  • modified: File "file2.pde" has been modified by another program, do you want to reload it?
  • modified both externally and in PDE: File "file3.pde" has been modified by another program, do you want to reload it and lose changes from Processing?
  • deleted: File "file1.pde" was deleted, keep it in editor?
  • added: Add file to sketch automatically. If it's in the folder, it's part of the sketch and will appear anyway when you open the sketch next time.
@benfry

This comment has been minimized.

Show comment
Hide comment
@benfry

benfry Jan 29, 2017

Member

That sounds right with minor changes:

  • modified externally & no modifications in PDE – just grab the new code from the disk. I've noticed that Eclipse has started to simply reload files that are changed externally without asking, which I think is good behavior—why pester the user?
  • modified externally & unsaved changes – “This file was modified by another program. Would you like to keep this version or load the new changes?” … whichever version loses can be saved as file3.pde.unsaved or something like that, just in case
  • deleted – "file1.pde" has disappeared from the sketch folder, would you like to re-save it or remove it from your sketch? with buttons "Re-save" and "Remove"
  • added – just add immediately, no dialogs
Member

benfry commented Jan 29, 2017

That sounds right with minor changes:

  • modified externally & no modifications in PDE – just grab the new code from the disk. I've noticed that Eclipse has started to simply reload files that are changed externally without asking, which I think is good behavior—why pester the user?
  • modified externally & unsaved changes – “This file was modified by another program. Would you like to keep this version or load the new changes?” … whichever version loses can be saved as file3.pde.unsaved or something like that, just in case
  • deleted – "file1.pde" has disappeared from the sketch folder, would you like to re-save it or remove it from your sketch? with buttons "Re-save" and "Remove"
  • added – just add immediately, no dialogs
@GKFX

This comment has been minimized.

Show comment
Hide comment
@GKFX

GKFX Jan 29, 2017

Contributor

@benfry Sounds good, I will implement if you guys haven't done it already.

Contributor

GKFX commented Jan 29, 2017

@benfry Sounds good, I will implement if you guys haven't done it already.

@JakubValtar

This comment has been minimized.

Show comment
Hide comment
@JakubValtar

JakubValtar Jan 30, 2017

Contributor

@GKFX Thank you, we're looking forward to it!

Contributor

JakubValtar commented Jan 30, 2017

@GKFX Thank you, we're looking forward to it!

@GKFX

This comment has been minimized.

Show comment
Hide comment
@GKFX

GKFX Mar 19, 2017

Contributor

Just to say that while I've left this inactive for a while, I am working on it again now.

Contributor

GKFX commented Mar 19, 2017

Just to say that while I've left this inactive for a while, I am working on it again now.

@benfry

This comment has been minimized.

Show comment
Hide comment
@benfry

benfry Apr 22, 2017

Member

@GKFX Thanks, let us know when it's ready for review!

Member

benfry commented Apr 22, 2017

@GKFX Thanks, let us know when it's ready for review!

@benfry benfry changed the title from Make it safe to add tab files to an open sketch. to Fix change detection when adding files to a sketch Apr 22, 2017

GKFX added a commit to GKFX/processing that referenced this pull request Apr 22, 2017

@GKFX

This comment has been minimized.

Show comment
Hide comment
@GKFX

GKFX Apr 22, 2017

Contributor

@benfry I've sent a new PR.

Contributor

GKFX commented Apr 22, 2017

@benfry I've sent a new PR.

@benfry

This comment has been minimized.

Show comment
Hide comment
@benfry

benfry Apr 22, 2017

Member

Ok, thanks! Closing this one.

Member

benfry commented Apr 22, 2017

Ok, thanks! Closing this one.

@benfry benfry closed this Apr 22, 2017

@GKFX GKFX deleted the GKFX:feature-safechangedetect branch Aug 29, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment