ci_edit icon indicating copy to clipboard operation
ci_edit copied to clipboard

Detect file changes

Open aaxu opened this issue 7 years ago • 33 comments

This PR includes a new object in the TextBuffer object that tracks the file on disk using threads. If in single-threaded mode, the object will not automatically track the file, but can be used to manually grab the information from disk. So far, this thread sends requests to the background thread in background.py to notify the main thread to do things. So far, the thread will detect if the file is "Read Only", and if the file has changed since it was last saved/opened. If the file was changed since it was last saved/opened, a popup box will appear, asking the user if they would like to reload the file. For some reason, there is a bug somewhere which is causing the popup box to not appear the screen refreshes again, but I'm putting this PR up to just get some input for now, as I work on it some more.

I made the popup box so it made it easier to see for the user to see, and I figured we could also make other important messages appear in this box, like saving or overwriting a file.

aaxu avatar Jan 06 '18 06:01 aaxu

Sorry for the slow feedback (on my part) for this PR. I'll try to get more time with it this evening or Monday.

dschuyler avatar Jan 19 '18 18:01 dschuyler

No worries! I've been taking a break from this PR because I just couldn't figure out the synchronization issue. Maybe I'll be able to give it another go this weekend.

Essentially the bug that is occurring is that sometimes, the popup window will not render on the screen, but the focus has changed to it. It will show up on the screen if any key is pressed (thus refreshing the screen and making it display). This could be troublesome because if the user types Y, they could accidentally reload the text file and lose all their current changes.

aaxu avatar Jan 19 '18 20:01 aaxu

WDYT of separating the popup window to another PR (that would make reviewing it easier)

dschuyler avatar Jan 22 '18 18:01 dschuyler

I could do that. It just seemed incomplete if I were to move the popup out of this PR and also I needed to test it using the popup, so I bundled them up together. I'll see if I can separate it out.

aaxu avatar Jan 22 '18 19:01 aaxu

This PR fixes #144 as well.

aaxu avatar Jan 27 '18 23:01 aaxu

I've just updated this to work with the new master branch

aaxu avatar Mar 05 '18 17:03 aaxu

@dschuyler Do you think there are any functionality changes that need to be made? It currently polls the file every ~2 seconds, but I don't think each poll has a lot of overhead. There's still that one synchronization bug, but I still have yet to figure out why it's happening.

aaxu avatar Mar 14 '18 15:03 aaxu

On Linux machines (and WSL), we can use inotify (native). For Python, there are also various platform-independent libraries.

Androbin avatar Mar 16 '18 16:03 Androbin

I looked into a different platform called watchdog which also used inotify. The issue with this is that when you do a git checkout, it doesn't modify the actual file, but I think it replaces it with another, so I couldn't see how to do it without watching the entire directory for changes. Implementing it like this also reduces the number of dependencies and would be easier to make it better suit our needs, so this was what I opted for, at least for a proof of concept. However, maybe we can move on to a different platform for different host OS for better efficiency ( I don't know if macOS has its own kind of inotify).

aaxu avatar Mar 16 '18 18:03 aaxu

@dschuyler are we moving away from using the Window.hide() function now? I see that there is now an assert False at the beginning of it.

aaxu avatar Apr 12 '18 18:04 aaxu

Yes, Window.hide() is transitioning. I had hide/show removing or adding the window to the parent. At the moment, detach()/reattach() play a similar role. I'm up for suggestions on what to call these things. The concept I'm trying to name is one where:

  • the parent does not see the sub-view as a child. It doesn't pass events to it and such. It's "detached" from the parent.
  • the sub-window still knows who it's parent is suppose to be. The sub-window will still pass requests up to the parent (even though the parent doesn't see it as a child). I'm not 100% sure what the best answer is here yet, but that's why .hide() is in flux.

dschuyler avatar Apr 12 '18 18:04 dschuyler

I actually have a question regarding the format of the program.

From what I understand, inputWindow is everything we see on the screen when we start up the program. This window contains a text buffer (which is where we display the file and type into the file), just like all other windows. However, this window also contains other windows like labeledLine, and lineNumbers.

Then, there is the programWindow which essentially contains the high level windows like inputWindows, palletteWindows, debugWindows, etc. I am guessing that when we decide to support split screen, we'll have multiple inputWindows? This programWindow also controls the zOrder and layering of all these high level windows. I don't think the inputWindows control or need to control any of its own windows.

Do I have the general idea correct?

aaxu avatar Apr 12 '18 19:04 aaxu

Yes-ish.

Think of Windows being in a hierarchy. The top-most window (or root window) is a ProgramWindow window.

... inputWindow is everything we see on the screen when we start up the program.

Yes, though all of that is within the ProgramWindow, but the ProgramWindow doesn't have any UI of it's own. ProgramWindow could have UI, it just doesn't. It's used as a container to stick other windows into.

This window contains a text buffer (which is where we display the file and type into the file), just like all other windows. However, this window also contains other windows like labeledLine, and lineNumbers.

Yes.

Then, there is the programWindow which essentially contains the high level windows like inputWindows, palletteWindows, debugWindows, etc. I am guessing that when we decide to support split screen, we'll have multiple inputWindows?

Yeah, that's the idea.

This programWindow also controls the zOrder and layering of all these high level windows.

Yeah. Actually every window can have child windows. The zOrder is the list of child windows. I've considered changing the member variable zOrder to children. Whatever it's called, it is a list of child windows ordered by their 'depth' in the Z axis (where X and Y are horizontal and vertical on the screen). The (fake) Z depth is done using the painter's algorithm (which just means we draw the things furthest away first and overlay the nearer things later, like a painter does when painting a scene on a canvas).

I don't think the inputWindows control or need to control any of its own windows.

I don't understand what this is saying, sorry. Could you say or ask this in another way?

Do I have the general idea correct?

Sounds like it, though I'm not sure on that last one.

dschuyler avatar Apr 13 '18 02:04 dschuyler

Trivia: Initially ci_edit didn't have a ProgramWindow. I was using the CiProgram as the topmost window. That got weirder and more cumbersome over time. Eventually I created ProgramWindow to give a cleaner top-level window and a cleaner separation between CiProgram and the window hierarchy. I'm glad I did. It's been helpful. So there should only every be one ProgramWindow and it mainly exists to keep the code organization cleaner.

dschuyler avatar Apr 13 '18 02:04 dschuyler

Bonus:

I printed out a hierarchy for ci_edit when it's first started and annotated what the pieces are:

<app.program_window.ProgramWindow -- root window, container for other windows
  <app.file_manager_window.FileManagerWindow -- the file path line
    <app.window.LabeledLine -- the blank line at the bottom
    <app.file_manager_window.DirectoryList -- the list of files
      <app.window.OptionsRow -- the header line in the list of files
    <app.window.OptionsRow -- the title line at the top
    <app.window.OptionsRow -- options like show dot files, etc.
  <app.window.InputWindow -- the main edit window
    <app.window.ViewWindow -- the upper left where it says "ci"
    <app.window.ViewWindow -- right hand column (one char wide)
    <app.window.LineNumbers -- left hand line number column
    <app.window.StatusLine -- line at the bottom that shows cursor row,col.
    <app.window.TopInfo -- top two lines that show context
    <app.window.InteractiveFind -- a container without its own UI
      <app.window.LabeledLine -- the "Find:" line
      <app.window.LabeledLine -- the "Replace:" line
      <app.window.RowWindow -- line with the options toggles
        <app.window.OptionsToggle -- "[x] regex"
        <app.window.OptionsToggle -- "[x] wholeWord"
        <app.window.OptionsToggle -- "[x] ignoreCase"
    <app.window.MessageLine -- line at very bottom (normally blank)

[edit: I'd mislabeled/omitted the right column view and TopInfo.]

dschuyler avatar Apr 13 '18 02:04 dschuyler

Oh what I meant was just that I thought it would have been better to have all the painting and layering done by the programWindow and not the inputWindow as well, but now I'm thinking each window should control its own subwindows to make it easier (like what we have right now).

aaxu avatar Apr 13 '18 19:04 aaxu

Also, what do you think about making some variables global, or adding a reference to the programWindow to all windows when initialized. I find that if I want to get access to the programWindow, or the popup window, I sometimes would have to do self.view.host.host or something. This makes it pretty confusing to keep track of what exactly you're referencing. If we end up going into a very deep subwindow, I could see a long list of self.view.host.host.host.....

aaxu avatar Apr 13 '18 19:04 aaxu

How about a global object windows mapping e.g. windows.edit_main to the app.window.InputWindow instance etc. using __getattr__ ?

Androbin avatar Apr 13 '18 20:04 Androbin

I'd like to avoid globals (even though I use them at times). Can the desired effect be done with this paradigm

# In a base class, such as ViewWindow.
def doFoo(self):
  self.parent.doFoo()

# In some parent, such as ProgramWindow:
def doFoo(self):
  reallyDoTheFoo()

dschuyler avatar Apr 13 '18 20:04 dschuyler

P.S. I also agree that self.view.host.host.host... is terrible. I intend to reduce that style.

dschuyler avatar Apr 13 '18 20:04 dschuyler

@dschuyler How about constructing windows dynamically from the current node? This doesn't require a global, just the depth of the current node (the number of .hosts).

Androbin avatar Apr 13 '18 20:04 Androbin

For example: self.windows.edit_main.do_something() where windows is also a dynamic attribute.

Androbin avatar Apr 13 '18 20:04 Androbin

How about constructing windows dynamically from the current node?

I'm not 100% sure I understand, sorry.

New note about parent vs host (trivia): I'd initially conflated parent and host and I should not have. A parent is the Window immediately above this window in the hierarchy. A host was intended to be the window that holds the current document. Often the host would equal the parent (so often that I conflated them), but the host could be several parents up.

Lately I've been thinking that host is not a good idea (at all). Instead it may be better to pass events (function calls) to self.parent.foo() until a parent designated to handle the situation handles it (i.g. that instance declares itself 'host' by virtue of handling the foo()) -- so the child window doesn't really know who the host is, but it can ask it's parent to pass up the message.

Goals: I'm looking for a solution that

  • is robust in the application and in unit tests (globals make problems in unit tests).
  • allows for changing the window hierarchy (having a child assume the host is N levels up is a problem for this)
  • allows having multiple document windows (globals are a problem here as well)
  • is a repeatable paradigm, i.e. the same way of doing it can apply in different situations (I want to avoid having different things do different paradigms).

What is in ci_edit right now does not reach those goals (my fault, it's something I'm working on fixing).

dschuyler avatar Apr 13 '18 20:04 dschuyler

For example: self.windows.edit_main.do_something() where windows is also a dynamic attribute.

Thanks, I'll think on that more.

dschuyler avatar Apr 13 '18 20:04 dschuyler

Would we have one inputWindow per file open / per view of a file open? I think this would make supporting multiple files easier since we wouldnt have to worry about switching out text buffers and resetting all the windows.

We can maybe store all open files as "inputWindow“s in memory. The program window can have a list of inputWindows to display, as well as a list of open but inactive (not being displayed) inputWindows (files/views). The program window can then have pre defined formatting for displaying 1 or 2 inputWindows, which can just be what we have right now, or split screen.

I also imagine having a "tabs" section (maybe a subwindow in programWindow) that shows open windows. Changing tabs would just mean swapping the programWindows displayed inputWindows with the clicked one.

Im thinking that we could make most windows recursively ask their parent for an action to occur, just as you mentioned before. This would solve the references for an ancestor N levels above.

aaxu avatar Apr 13 '18 22:04 aaxu

Just some notes that will hopefully clarify the current intent.

For open files and InputWindows the intent is to allow for many open TextBuffers and many InputWindows.

Any given InputWindow will have exactly one TextBuffer. If there are no open TextBuffers from disk, an empty (new) TextBuffer is created for the InputWindow.

A TextBuffer can exist without an InputWindow. E.g. a TextBuffer can be loaded, manipulated, and written back to disk without ever appearing in an InputWindow (this doesn't currently happen, but the design is that it could be done).

The TextBuffer that an InputWindow is looking at can be changed (this happens every time a new file is viewed without quitting the editor, since there's only one InputWindow currently)*.

In the future, there may be many InputWindows. Any InputWindow may be viewing any of the TextBuffers.

Two or more InputWindows may be using the same TextBuffer. Edits made in either InputWindow will appear in the other.

If tabs are implemented*, the TabRowWindow (or whatever it may be called) would likely be a child of the ProgramWindow. The InputWindows may be either siblings of the TabRowWindow (makes sense if there is a single tab controller); or they may be children of a TabRowWindow (makes sense if there may be multiple tab controllers).

*The currently viewed TextBuffer within an InputWindow can be changed with ctrl+p.

dschuyler avatar Apr 16 '18 18:04 dschuyler

Right, so for right now, should we look towards refactoring a bit to support this paradigm? If we are going to change the hierarchy and relationships between windows, it would be easier to do it earlier than later.

Maybe for now we can do something like the following:

  • Make windows only talk to their direct parent/children
  • Add functions that would allow windows to ask for the program window and inputwindow

This would make it easier to:

  • Finish this PR and make it function without making many assumptions about the hierarchy

Also one more thing. Should we add the inputWindow's textBuffer into its own window (FileContentWindow)? It seems like the text buffer object is just floating around in the inputWindow while being surrounded by other windows, though I don't know how necessary this new window would be.

aaxu avatar Apr 16 '18 18:04 aaxu

Maybe for now we can do something like the following: Make windows only talk to their direct parent/children

This is my current goal (though the code isn't there yet)

Add functions that would allow windows to ask for the program window and inputwindow

My hope is that passing messages up parent to parent will alleviate the need for asking for the program window or input window.

dschuyler avatar Apr 17 '18 01:04 dschuyler

Make windows only talk to their direct parent/children

So our paradigm is the Mediator Pattern, isn't it? The Polymer docs phrase it like this:

Polymer implements the mediator pattern, where a host element manages data flow between itself and its local DOM nodes.

When two elements are connected with a data binding, data changes can flow downward, from host to target, upward, from target to host, or both ways.

When two elements in the local DOM are bound to the same property data appears to flow from one element to the other, but this flow is mediated by the host. A change made by one element propagates up to the host, then the host propagates the change down to the second element.

Androbin avatar Apr 17 '18 14:04 Androbin

Yes that's the paradigm I'm following. I'm trying to work a bit on refactoring this. Is a view just the name for what an object calls the Window object that it is contained in? For example, textBuffer.view will always reference a window object.

aaxu avatar Apr 21 '18 03:04 aaxu