Skip to content
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

Python editor should detect externally modified scripts #139

Closed
RenWal opened this issue Jun 26, 2019 · 2 comments · Fixed by #170
Closed

Python editor should detect externally modified scripts #139

RenWal opened this issue Jun 26, 2019 · 2 comments · Fixed by #170
Assignees
Labels
Priority: High Needs to be addressed as soon as possible Status: Needs Review Waiting for available reviewer to pick up this issue or PR Type: Bug This is a confirmed or unconfirmed bug

Comments

@RenWal
Copy link
Contributor

RenWal commented Jun 26, 2019

Describe the bug
The Python editor currently does not monitor the file on disk, meaning if you modify the file in another editor (or you have it in, say, a git repository and pull in some commits) and then click the save button in HAL, the external changes are lost.

Most IDEs can do this, so this will probably be unexpected for most users.

To Reproduce

  1. Create a Python script in HAL and save it.
  2. Open the script in an editor of your choice and change something.
  3. Hit save in HAL.
  4. Open the script in the external editor again and see that your changes have been overwritten.

Expected behavior
The editor must not overwrite a file that has been modified externally without asking for confirmation. The editor should display a subtle notification (no focus-stealing dialogs here) asking whether the user would like to re-load the file whenever HAL senses an external modification.

Additional context
gedit does this well, maybe take a look there.
gedit-file-changed

@issue-label-bot issue-label-bot bot added the Type: Bug This is a confirmed or unconfirmed bug label Jun 26, 2019
@issue-label-bot
Copy link

Issue-Label Bot is automatically applying the label Type: Bug to this issue, with a confidence of 0.59. Please mark this comment with 👍 or 👎 to give our bot feedback!

Links: app homepage, dashboard and code for this bot.

@dertob dertob self-assigned this Jun 26, 2019
@dertob dertob added the Status: In Progress Somebody is working on this label Jun 26, 2019
@swallat swallat added the Priority: High Needs to be addressed as soon as possible label Jun 28, 2019
@dertob dertob added the Status: Blocked Other work needs to be done first label Jun 30, 2019
@dertob
Copy link
Contributor

dertob commented Jun 30, 2019

Detection of externally modified, moved or deleted scripts is working in branch 'feature/python-editor-file-modified'. The user is presented with the choice to reload a file from disk or ignore the external changes.

The only thing missing in this feature is that tabs need to be marked as modified as soon as

  1. the modification on disk gets ignored by the user.
  2. the script has been moved on disk.
  3. the script has been deleted on disk.

Due to #157 tabs cant be marked as modified with 100% assurance though. I suggest fixing #157 first instead of merging this right now.

Do not merge. Needs some adjustments after graph view integration and changes on the global file status manager.

@dertob dertob removed the Status: In Progress Somebody is working on this label Jul 1, 2019
@dertob dertob added Status: In Progress Somebody is working on this and removed Status: Blocked Other work needs to be done first labels Jul 10, 2019
@dertob dertob added Status: Needs Review Waiting for available reviewer to pick up this issue or PR and removed Status: In Progress Somebody is working on this labels Jul 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: High Needs to be addressed as soon as possible Status: Needs Review Waiting for available reviewer to pick up this issue or PR Type: Bug This is a confirmed or unconfirmed bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants