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

Edit/Redo stops working #192

Closed
marilmanen opened this issue Dec 8, 2020 · 34 comments
Closed

Edit/Redo stops working #192

marilmanen opened this issue Dec 8, 2020 · 34 comments

Comments

@marilmanen
Copy link
Contributor

I've encountered a new issue that I haven't seen ever before.
I was modifying a text file (with version 2020.1-68-gb117490) and suddenly redo (Ctrl+Shift+Z) stopped working although it had worked many times already with the same file. Issue is visible only in one of the files that I have open in my tabs (in two separate window created with detach tab). In the failing tab the file the status does not change even if I modify the content either by adding/removing text or by using undo (Ctrl+Z) and the Edit/Redo menu item remains grayed out. The undo command works also in a strange way in that tab i.e. it undo's the old changes that I did when the redo was still working and does not undo any of the changes that I do now. I'll continue working with a new NEdit-ng server and leave the broken server running just in case you need some more information.

@marilmanen
Copy link
Contributor Author

Unfortunately I accidentally activated the failing window and due to modifications done in the other NEdit server the corrupted file gave pop-up window "has been modified by another program. Reload?" and that reset the issue and Redo works now again.

@eteran
Copy link
Owner

eteran commented Dec 8, 2020

Interesting. Can you describe your environment? OS/Distro, Qt version, compiler version, desktop, etc? Running via nc-ng or nedit-ng directly?

You seem to encounter issues that I don't see locally, so I'm just trying to figure out what the difference is.

Thanks!

@eteran
Copy link
Owner

eteran commented Dec 8, 2020

Also, is this an edit session where you've had nedit open for a LONG time? Did you use lots of macros? Hopefully, we can figure out what the trigger is.

@marilmanen
Copy link
Contributor Author

I took latest code into use on Dec 5, and started working, so I would like to think that this is not a long time. I use macros heavily, so I would estimate that during these 3 days I must have executed >100 macro commands. Most of the macros execute on the background and modify some of the text files that are also open, so I get a lot of intentional "has been modified by another program. Reload?" pop-up messages. The file for which Undo stopped working was not one of the files that is externally modified by my macro commands.
First NEdit session was started with nc-ng command and I'm using in CentOS 7.7.1908

@marilmanen
Copy link
Contributor Author

I have now started to see some odd "has been modified by another program. Reload?" pop-up messages, so could you clarify when these messages are expected? Only when a tab with externally modified file is activated, or for some reason at some other time also (I don't think it should)? At the moment it looks like I get a pop-up message based on the activated tab index and not based on which of the detached window the tab is activated. I have tried to reproduce the issue with dummy files with which I have not already worked for two days, but I have not succeeded.

@marilmanen
Copy link
Contributor Author

Now the issue has escalated to a situation where tab name is not the same anymore as the actual file name and I have no idea at which time the tab name changed. In the Windows menu I have still the correct tab names, but the tab bar shows wrong name for one of the files (two times same name in the tab bar). I'll switch back to the April version as I think it was better.

@eteran
Copy link
Owner

eteran commented Dec 10, 2020

Wow, OK, I'll continue to look into things and see if I can replicate these things. I can say that your usage pattern of using macros to intentionally trigger reload messages is very different than what I do, so hopefully I can figure out what the cause of these things are!

@eteran
Copy link
Owner

eteran commented Dec 21, 2020

@marilmanen when you get a chance, can you provide any updates on this?

You recently said in another thread that you had an issue with your setup. Is it possible that some of the issues you describe where either environmental or otherwise addressed by recent updates?

@marilmanen
Copy link
Contributor Author

My setup issue was related to a shared tool setup file where all tool versions are selected. My intent was to freeze nedit-ng to some old version for all other accounts and use latest version only with my account, but due to a mistake also my nedit-ng version was the old version. Now I'm using the latest and none of my trials should be visible to others. I have planned to make some systematic issue hunting, but it keeps on moving forward in time.

@eteran
Copy link
Owner

eteran commented Dec 22, 2020

I can understand how it's a moving target! But hopefully, nedit-ng is rapidly approaching the "rock solid" status we want it to be :-)

@marilmanen
Copy link
Contributor Author

In some mysterious way I managed to get the Redo to be greyed out permanently again and the symptoms are exactly the same as in my original comment for this issue.
image

In this NEdit window I have used my macro command that removed the file that was open on the same tab from which the macro was executed (I have not used that macro many times lately), so it may have something to do with the issue. At least I have a feeling that this macro has always been used before strange issues start...

result = dialog("Really sure to remove " $file_name, "no", "YES" )
if ( result == 2 ) {
  shell_command("rm " $file_name, "")
  result = dialog($file_name " has been removed, close window without saving to continue" )
  focus_window($file_name)
}

Purpose of the focus_window() is to ensure that the file removal is noticed by NEdit, but sometimes it does not give the "File not Found" pop-up message, so I wonder if that's an indication that something goes wrong. I'm using NFS, but it should not have any impact.

I assume that also this time the issue will escalate and thus I'll restart the NEdit as I don't trust it anymore.

Version is 2020.1-207-gcee33fb

@eteran
Copy link
Owner

eteran commented Sep 8, 2021

OK, the macro is hopefully a big hint. Maybe there's some weird circumstance where the shell_command somehow gets stuck and things get into a strange state.

Both the shell_command and dialog are callback-based, so I can imagine (and hopefully find) some circumstances where they suddenly make no progress. We'll get to tyhe bottom of this one!

@marilmanen
Copy link
Contributor Author

Now I have the "Redo to be greyed out" issue again and this time I have not used the file removal macro at all. I'm still using a lot of other macro commands so next possible cause is that while running a macro command in one tab I have removed another tab. Could that cause some issue?

@eteran
Copy link
Owner

eteran commented Oct 25, 2021

That could DEFINITELY be a factor! This could be a very big clue for where I should look for this fix :-)

@marilmanen
Copy link
Contributor Author

Now I have seen this issue again two times this week with version 450ad17. I have not managed to figure out what is the sequence to reproduce the issue, but at the moment I have one NEdit window in which one of the 16 tabs is having the issue. Is there any debug information I could provide? I tested "Shell/Execute Command Line" and it works ok. Also Macro command dialog($file_name, "ok") returns correct file name.

@eteran
Copy link
Owner

eteran commented Jan 19, 2024

Hmm, it's been a while since I've looked at this code. But maybe I can spot something after looking at it with fresh eyes.

@marilmanen
Copy link
Contributor Author

Now I have finally found a way to reproduce the undo/redo buttons to be inactive. (With Logitech 502 settings this is an easy mistake if middle button is configured to G8)

  1. select text
  2. move selected text with mouse middle button and at the same time press left mouse button.
  3. try using undo/redo commands

I tested also with old NEdit and it behaved in the same way.

@eteran
Copy link
Owner

eteran commented Jul 8, 2024

oh wow! with this new info I'll see if I can reproduce myself and come up with a fix. Interesting that (if I understand correctly) that it is even present in the original nedit classic!

@eteran
Copy link
Owner

eteran commented Jul 8, 2024

confirmed! I don't have any idea what's going on but yes, i was able to reproduce the issue. Something is definitely detecting an inconsistent state when it happens because I do get the pc speaker beep to indicate that something went wrong, but the result is undo/redo is broken.

I will get to the bottom of this now that I have something to test that's repeatable.

Thanks

@eteran
Copy link
Owner

eteran commented Jul 9, 2024

@marilmanen good news, I know what's wrong, I just need to identify the best solution.

Thanks for helping track it down!

eteran added a commit that referenced this issue Jul 10, 2024
So if you the user is dragging with a middle mouse button, then right and left clicks are ignored.
Fixes (long standing) issue #192 ... I hope!
@eteran
Copy link
Owner

eteran commented Jul 10, 2024

@marilmanen OK... I think I have a fix!

#357

Please give this branch a try and see if it fixes the issue (as far as you can tell). This one was a subtle one for sure. Basically it was possible to corrupt the "state machine" used to track mouse drag states by interleaving event types. To be honest, I'm not 100% sure why that would result in the undo/redo menus being broken, but by preventing it from getting into an inconsistent state, it seems to prevent that from happening in my tests.

Let me know!

@marilmanen
Copy link
Contributor Author

Looks like the fix works as expected as I'm not able reproduce any issues after trying many combinations with different mouse key, Ctrl and Shift keys combinations. I'm afraid that this might be the last issue I'll find, but let's see...

Thank you again for you very fast response time.

@marilmanen
Copy link
Contributor Author

Hmm, there might be an issue with the fix as I don't get the pop-up window with right mouse button anymore.....

@eteran
Copy link
Owner

eteran commented Jul 10, 2024

OK, I'll take a look. I expected it to still work, just only disabled if you happen to be already mid click/drag. I did that because it was yet another way to cause the issue.

Maybe I can come up with a middle ground, like only one drag operation at a time or something.

@eteran
Copy link
Owner

eteran commented Jul 10, 2024

Thought about it, I think I know what's going on. I have some code in there that will track the mouse state and just reject some events.

Like if you're mid-drag it will reject the context menu event. This is all done in the mouse up/mouse down handlers.

I bet that when i test on Windows, these events fire in one order: Context Menu then Mouse Press then Mouse Up...
and that on Linux they happen in anther order: Mouse Press, then Context Menu then Mouse UP.

So if I block the even in Mouse Press, Context Menu gets rejected.

I'll have to think more about the solution after some testing to confirm.

@eteran
Copy link
Owner

eteran commented Jan 7, 2025

@marilmanen Happy New Year! So I think that I may have gotten the fix right this time. It was indeed a subtle difference between how the OSes handle right clicks with regard to triggering context menus. So after a bunch of thinking and a bunch of random ideas that didn't work, I just decided to disable the OS level context menu triggering and just do it manually and ... I think it's working correctly!

Can you give the latest version of #357 a try to see if it helps?

If so I'll merge it, and then will do a 2025.1 release

(Just as a heads up, this will be the last release that uses C++14 as I've been wanting to use some C++17 stuff and I think 7 years i long enough to consider it widely supported).

Thanks!

@grege2
Copy link

grege2 commented Jan 7, 2025

C++17 ? Yikes.
I learned C++ reasonably well in about 1992. I guess I never got to grips with templates and multiple inheritance and few other things. But I had a test program that used every piece of C++ I knew. Over the years it "crumbled", C++ got tougher and pieces wouldn't compile, wouldn't run correctly. The last I checked hardly a single line would compile. C++17 would destroy it I'm sure. "The good thing about standards is - there are so many to choose from".

@marilmanen
Copy link
Contributor Author

Happy New Year.

I have taken the fixed version into use and it looks like the issue is resolved or at least I have not yet found any issues :-)

@marilmanen
Copy link
Contributor Author

One minor thing with the new version. Right mouse is now working in a bit different way for the macro pop-up window.
Previously I could just press the right button down and release it on any macro that I wanted to use, but now I must make a whole click to get the pop-up window open and a second click to run some macro.

@eteran
Copy link
Owner

eteran commented Jan 8, 2025

C++17 ? Yikes. I learned C++ reasonably well in about 1992. I guess I never got to grips with templates and multiple inheritance and few other things. But I had a test program that used every piece of C++ I knew. Over the years it "crumbled", C++ got tougher and pieces wouldn't compile, wouldn't run correctly. The last I checked hardly a single line would compile. C++17 would destroy it I'm sure. "The good thing about standards is - there are so many to choose from".

LOL, well to be fair the first C++ standard was in 1998 and then updated in 2003. So you were working with a pre-standard (and thus subject to change) version of the language. I don't know if you follow it much, but C++20 is already considered "old" since it's been out for nearly 5 years and C++23 was just released about a year ago. There's actually a large amount of contention in the C++ community as to when it's ok to break different types of backwards compatibility since the committee REALLY doesn't want to break existing code.

(They got burned mandating in C++11 that std::string not be reference counted thus forcing gcc to change its implementation, so gcc had to ship a library that somehow supported BOTH implementations seamlessly).

nedit-ng already compiles fine as C++17 code, I just want to be able start using new features and drop the dependency on boost since the only things I use boost for... are now part of the C++-17 standard!

So yeah, I think aside from the README saying that gcc 7 will be required to build it, I think nothing else will change on the project ;-)

@eteran
Copy link
Owner

eteran commented Jan 8, 2025

One minor thing with the new version. Right mouse is now working in a bit different way for the macro pop-up window. Previously I could just press the right button down and release it on any macro that I wanted to use, but now I must make a whole click to get the pop-up window open and a second click to run some macro.

Interesting. To be clear, you're saying that you used to be able to:

  1. right click
  2. the menu would pop up THEN
  3. move the mouse to an item on context menu
  4. release

and then it would select the item?

I had experiemented with triggering the context menu on mouse down, but had run into an issue where the popup menu actually physically prevented the mouse up event on the text area, but maybe that's what you wanted, LOL.

Out of curiosity, do you know what the behavior of the original nedit was in this regard?

(I also just noticed that by accident right click and drag now triggers the scrolling feature even without Ctrl pressed, so that's a bug that will hold up the merge anyway).

@eteran
Copy link
Owner

eteran commented Jan 8, 2025

@marilmanen try it now 🤞🏻

@marilmanen
Copy link
Contributor Author

Yep, now it works perfectly

Thanks again

eteran added a commit that referenced this issue Jan 8, 2025
* Making it so mouse interaction only tracks one mouse button at a time.
So if you the user is dragging with a middle mouse button, then right and left clicks are ignored.
Fixes (long standing) issue #192 ... I hope!

* mostly cleaning up includes

* i think that this actually solves the issue!

* this code can be static to avoid redundant work

* maybe need to update boost for windows builds

* fix for additional quirk that @marilmanen noticed!
@eteran
Copy link
Owner

eteran commented Jan 8, 2025

OK, merged in! thanks for your help tracking this one down, it was a doozy! That was the last issue I felt was holding up the next release, so I plan to do that this week.

@eteran eteran closed this as completed Jan 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants