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

Open Files Do Not Update After SVN Update #633

Closed
JoelProminic opened this issue Nov 5, 2019 · 15 comments
Closed

Open Files Do Not Update After SVN Update #633

JoelProminic opened this issue Nov 5, 2019 · 15 comments
Assignees
Labels
bug in-progress test ready Feature/bug ready for testing

Comments

@JoelProminic
Copy link
Contributor

I have been using Moonshine for a Java SVN project recently, and I ran into this bug after working on another workstation:

  1. Open an SVN project in Moonshine
  2. Open a file
  3. Update the same file in another copy of the SVN project and commit
  4. In Moonshine use Subversion > Update. The console shows the update
  5. BUG: The updates do not show in the open tab
  6. Make an edit in the tab and save
  7. BUG: The updated file is overwritten by the saved contents of the editor

We don't have a way to diff the changes to a file from Moonshine currently, so it is not immediately obvious that the changes were overwritten.

This same error may trigger for Git or file changes from another local editor.

I'd like to see some protection against this behavior. For example:

  1. Automatically update the tabs for a project after an SVN update
  2. Warn the users that they need to reopen the tabs
  3. When saving, check if the file timestamp has changed since the last save and prevent the user from saving. I think this would be the most generally useful solution, if this bug also affects Git and local file changes.
@JustinProminic JustinProminic added this to the v2.6.0 milestone Jan 21, 2020
rat-moonshine pushed a commit that referenced this issue Feb 3, 2020
… and provide alert/notification

- New command hooked on tab active/clicked
- Command added to test when SVN, Git update/pull
(reference #633)
rat-moonshine pushed a commit that referenced this issue Feb 3, 2020
- Added file-modification-test hook into SVN, Git update/pull
(reference #633)
@rat-moonshine
Copy link
Collaborator

rat-moonshine commented Feb 11, 2020

The current updates also looks into other text-editors and Eclipse, and I planned the steps in following way:

  1. You may have opened several tabs from a project
  2. Edited in tabA, tabC, tabE but none have saved
  3. Your current selection is tabA
  4. You do a Git/SVN pull/update
  5. You have new versioned files downloaded for tabA and tabC
  6. Moonshine will immediately show you this alert against your currently selected modified tabA:

image

  1. You choose to reload or keep the modification as they are
  2. Moonshine will not alert for any other modified tab immediately as they doesn't makes sense if you do not return to them at all during the session
  3. If you changed to tabC, it'll again display you the same alert

After every Git/SVN pull/update Moonshine validates currently opened editor's open timestamp with the physical file's modified timestamp. If the physical file's modification timestamp is newer, Moonshine will raise the alert.

This behavior is also true when you active any inactive tab. Checking while activating a tab doesn't requires Git/SVN pull/update dependency. It'll be automatic.

@JoelProminic raised some observation after above:

  • Will this cover the case where a file is edited locally by another application?
  • The Git files can also be updated by a Switch to Branch action
  • If the user updates a file multiple times before checking the tab, how will it handle the multiple updates?

We need to raise check in case of git-switch too (not done yet), and alert if the opened file does not exists in changed branch anymore.

rat-moonshine pushed a commit that referenced this issue Feb 12, 2020
…nst opened tab

- Adjustment to the editor-popup notification sizing
(reference #633)
rat-moonshine pushed a commit that referenced this issue Feb 12, 2020
@rat-moonshine
Copy link
Collaborator

  • Update check added upon branch-switch and new-branch-switch cases
  • Testing of physical file existence also added against opened/currently-selected tab/editor, shall offer two main options - re-Save and Close

@JoelProminic
Copy link
Contributor Author

I did some testing today with a non-trivial Java project, and I found I was getting false alerts from the new system. I couldn't identify the exact case where this triggered, but here are some possibilities:

  • I had multiple files open every time this happened, and I was seeing the alert in all tabs.
  • It seemed to trigger sometimes a few seconds after I had created a Java class.
  • Regardless of which option I chose, I kept getting this error until I closed all tabs.

I'll let you know if I have a more specific test procedure.

@rat-moonshine
Copy link
Collaborator

I confirm @JoelProminic reported problem when edited multiple files in Moonshine (not saved) and both file changed externally. Seems like a looped case, prompting by separate notifications:

image
image

Looking into this.

@rat-moonshine
Copy link
Collaborator

@joshtynjala can you help me find where this prompt is firing from? With my quick string search throughout the project, I didn't found any reference, surprisingly. This generally pop-up when opening a .java file to editor:

image

We have a need to tuneup showing the prompt as they overlaps each other at same time, when opening multiple .java project during Moonshine start.

rat-moonshine pushed a commit that referenced this issue Feb 14, 2020
@joshtynjala
Copy link
Collaborator

It probably comes from the Java language server. Language servers have the ability to ask the IDE to display a message.

@rat-moonshine
Copy link
Collaborator

It probably comes from the Java language server. Language servers have the ability to ask the IDE to display a message.

Can we adjust the timing of the alert that multiple do not overlaps against each opened .java editors, when Moonshine opens (?)

@rat-moonshine rat-moonshine added the test ready Feature/bug ready for testing label Feb 18, 2020
@joshtynjala
Copy link
Collaborator

We probably need to create some kind of queue where, if one of these message boxes is already open, any additional messages are stored until the current one is closed.

For reference, I'm pretty sure that this pop-up message is coming from here in JavaLanguageServerManager.language__actionableNotification:

https://github.com/prominic/Moonshine-IDE/blob/master/ide/MoonshineDESKTOPevolved/src/actionScripts/languageServer/JavaLanguageServerManager.as#L566

Maybe we can modify this StandardPopup component to have some kind of static method like StandardPopup.addToQueue() that manages the queue internally.

@rat-moonshine
Copy link
Collaborator

rat-moonshine commented Feb 19, 2020

We were infact discussing on an enhanced queued but noticeable-also notification system with @piotrzarzycki21 for some time, which generally mimic how notifications generates in editors like VS Code or IntelliJ. This seems will support the scenario you described also, Josh.

@rat-moonshine
Copy link
Collaborator

This issue now merged into master.

@JoelProminic
Copy link
Contributor Author

I did some testing with this today, and noticed this behavior:

  1. Open/Create a project in Moonshine
  2. Open a file
  3. Open the same file in an external editor
  4. Update the file in an external editor
  5. Change the focus to Moonshine - an alert appears
  6. Repeat steps 4 and 5 without accepting the prompt. BUG: another alert appears on top of the first
  7. Change focus between Moonshine and another application. BUG: a new alert appears each time
  8. Click No. BUG: Error [Domino Visual Editor] Interface Similar to "Objects" from Domino Designer #1009 appears on the console
  9. Click Yes. BUG: Error [Domino Visual Editor] Interface Similar to "Objects" from Domino Designer #1009 appears on the console
  10. Focus on Moonshine and check the menu (macOS). BUG: there are no menu drop-downs except for the standard Apple menu

Here is an example of the error:

: Click here to Report a Bug
: Error #1009
: TypeError: Error #1009
: 	at actionScripts.ui.menu::MenuPlugin/ensureHideUnhideMenuOption()
: 	at actionScripts.ui.menu::MenuPlugin/applyNewNativeMenu()
: 	at actionScripts.ui.menu::MenuPlugin/onMacEnableStateChange()
: 	at flash.events::EventDispatcher/dispatchEvent()
: 	at actionScripts.controllers::UpdateTabCommand/cleanUp()
: 	at actionScripts.controllers::UpdateTabCommand/seeFileAgain()
:
: Click here to Report a Bug
: Error #1009
: TypeError: Error #1009
: 	at actionScripts.ui.menu::MenuPlugin/ensureHideUnhideMenuOption()
: 	at actionScripts.ui.menu::MenuPlugin/applyNewNativeMenu()
: 	at actionScripts.ui.menu::MenuPlugin/onMacEnableStateChange()
: 	at flash.events::EventDispatcher/dispatchEvent()
: 	at actionScripts.controllers::UpdateTabCommand/cleanUp()
: 	at actionScripts.controllers::UpdateTabCommand/yesButtonTab()
:
: Click here to Report a Bug

I also see this behavior. I don't think it is necesarily wrong, but I'm considering whether it is desireable behavior for the use case:

  1. Get an alert for a changed file.
  2. Click No on the prompt to keep the in-memory changes
  3. Switch to a different tab
  4. Switch back to the original tab. The alert appears again. This could be annoying, but it also prevents the user from forgetting that the editor is out of synce
  5. Click No again
  6. Make a change to the editor and save again. There is no prompt in this case, so the user could overwrite updates from SVN or Git at this point. The user already chose to keep the local changes, though, so this is not unreasonable behavior.

@rat-moonshine
Copy link
Collaborator

Normal

  1. Open/Create a project in Moonshine
  2. Open a file
  3. Open the same file in an external editor
  4. Update the file in an external editor
  5. Change the focus to Moonshine - an alert appears
  6. Repeat steps 4 and 5 without accepting the prompt. FIX: Tab has no duplicated alert
  7. Click No. FIX: No error shown up on console
  8. Navigate to other opened tab(s)
  9. Returned to non-sync tab again. Moonshine alerts on file sync problem

File Does Not Exists Anymore

  1. Open/Create a project in Moonshine
  2. Open a file
  3. Manually remove the file by navigating through Finder/Explorer
  4. Return to Moonshine - an alert appears pointing the file is not exist anymore and provides relevant options

File Does Not Exists With More Cases

  1. Open/Create a project in Moonshine
  2. Open a file
  3. Open the same file in an external editor
  4. Update the file in an external editor
  5. Change the focus to Moonshine - an alert appears
  6. Close the file in external editor. Manually remove the file by navigating through Finder/Explorer
  7. Return to Moonshine - an alert appears pointing the file is not exist anymore and provides relevant options
  8. Click on Yes. Moonshine throws an error in its console: ": Unable to access file:/Users/..."
  9. Repeat 2-7 again
  10. Click on No. Moonshine does nothing
  11. A save keypress (CMD/Ctrl+S) re-save the file to its opened destination

@rat-moonshine rat-moonshine added the test ready Feature/bug ready for testing label Mar 3, 2020
@piotrzarzycki21
Copy link
Collaborator

I see this working on master - Can we close that ?

@rat-moonshine
Copy link
Collaborator

Perhaps, let's open this for some time. Cause some tests are still pending from Joel's side.

@piotrzarzycki21
Copy link
Collaborator

Perhaps, let's open this for some time. Cause some tests are still pending from Joel's side.

I was discussing this with you yesterday and Joel as well - Let's not keep things open. Let's close it and when release comes - everything will be captured in CHANGELOG - Joel can do review then. Repository becomes messy and unreadable cause we are holding opened issue for weeks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug in-progress test ready Feature/bug ready for testing
Projects
Development

No branches or pull requests

5 participants