Do I need to override revertToSaved ?

As John Daniel has said, NSDocument can be a real head scratcher. I had a hard time with the revert code when I first started using it. In addition to all the suggestions given, I would suggest putting NSLog statements in various places so you can see what is executing and in what order--in the document and windowController inits, reads, etc. I was a bit surprised when I did this to see that the order I had expected was not what was happening.

Might I also suggest overriding revertToContentsOfURL something like this:

-(BOOL)revertToContentsOfURL:(NSURL *)url ofType:(NSString *)type error:(NSError **)outError {

  BOOL success = [super revertToContentsOfURL:url ofType:type error:outError];
  if(success) {
  NSData *restoreData = [NSData dataWithContentsOfURL:url]; // get the data that we want to use for the restore
     
  // pseudocode
     
  process the restoreData the same way you would for a regular document read based on the type
  do anything you normally need to do with your windowcontroller

  } else {
  return NO;
  }
  return success;
}

This is bascially what I do and it works well. Sorry for the Objective C code--it's all I do.

I don't override the revertToSaved IBAction.

Thanks for this: I'll see if I can swiftify it. But I think the problem is that I haven't subclassed NSDocument sufficiently with KVO stuff, so "do anything you normally need to do with your windowcontroller" is the tricky bit, as I can't refresh the view.

Revert is working on the Data level, but not on the View level.

I can reload the view with the following code:

var viewController: ViewController? {

return windowControllers[0].contentViewController as? ViewController

}

override func revert(toContentsOf url: URL, ofType typeName: String) throws {

do {

try super.revert(toContentsOf: url, ofType: typeName)

viewController?.viewDidAppear()

}

}

But I get a log message telling me that I shouldn't load a view that's already loaded. I can't find a method for REloading the viewcontroller.

Trying to run setNeedsDisplay on a particular view of the viewcontroller didn't work.

hi,

sorry i missed the earlier back-and-forth (interesting stuff, to be sure -- and John's right about some of the head-scratching needed to work with NSDocument), but let me offer this as a general strategy, assuming you like the notion that an NSDocument is the "model" of the MVC design:

(1) when the NSDocument completes the "data" portion of its revert, have it broadcast a notification that it has just been reverted.

(2) have all your ViewControllers sign up for this notification

(3) when a ViewController receives this notification, have it update each of its views (calling setNeedsDisplay on each view will usually do the trick).

this is more or less what works for me [i have an NSDocument in which i am not using the UndoManager, and i never call super.revert() at any time].

one note: if you have more than one document open at a time, you'll have to check a notification when received that your ViewController belongs to the NSDocument that sent the notification.

hope that helps,

DMG

Thanks. Yes, I'm likely to have more than one document open, so that's going to be an issue. I've been looking at Stack Overflow:

https://stackoverflow.com/questions/44561256/how-to-reload-or-update-a-viewcontrollers-view-once-a-file-is-saved

which provides some swift code for notifications.

Could I not just have a binding from my PDFView to the PDFDocument?

hi,

usually, linking views directly back to the "model" is not a goood strategy in the MVC paradigm; but one could argue that you're just setting a "data source" for the view (e.g., UITableViews have a dataSource as well as a delegate). it's your design, and it will work for you until it doesn't work anymore, depending on the complexity of the app.

as for the SO thread, one thing to suggest is that when your document posts a notification that it has been reverted, it send along itself as the object of the notification. that way, anyone who receives the notification can tell right away if it came from their own document.

hope that helps,

DMG

I don't understand how ViewController can talk about documents but Document can't talk about views.

In ViewController.swift, I have:

var document: Document? {
         return self.view.window?.windowController?.document as? Document
     }

which I use so that the methods in VC can do things like document.updateChangeCount. Or should I be using Notifications for that?

Can I use that to help with identifying the correct document that sent the Notification?

Back in Document.swift, I now have:

        NotificationCenter.default.post(name: Notification.Name("Revert!"), object: Document.self)

in my revert function. Over in ViewController.swift viewDidLoad, I have:

        NotificationCenter.default.addObserver(self, selector: #selector(updateUI), name: Notification.Name("Revert!"), object: Document.self)

and then elsewhere:

@objc func updateUI() {
        thePDFView.setNeedsDisplay(thePDFView.bounds)
    }

But still nothing changes. (God these code boxes are awful.)

hi,

let me offer the following code snippets and suggestions for you -- the one thing that's wrong right now are the arguments to object: in post() and addObserver().

// make a name for this particular notification somewhere central in your app
// i have these names in a file called NotificationName+Extensions.swift
extension Notification.Name {
  static let documentReverted = Notification.Name(rawValue:"Revert!")
}

// in your Document, post the notification this way (note argument to object:)
NotificationCenter.default.post(name: .documentReverted, object: self)

// in your ViewController, add the observer this way in viewDidLoad
NotificationCenter.default.addObserver(self, selector: #selector(handleDocumentReverted), name: .documentReverted, object: nil)

// and respond to the notification this way in your viewController
@objc func handleDocumentReverted(_ note: Notification) {
  let sendingDocument = note.object as? Document
  guard sendingDocument == document else { return }
  thePDFView.setNeedsDisplay(thePDFView.bounds)
}

i have changed the argument of object: in the addObserver call to nil. you can set it to object: document as far as i can see (not object: Document.self), which automatically filters out observing documents that aren't yours. but it might be useful to see the logic and how, more generally, you can work with the object that came in with the notification.

and i hate the code boxes as well ...

hope that helps,

DMG

Thanks. I put a log message into the handle function, after the setNeedsDisplay, and it logs, but still the view remains unchanged.

If I save the PDF, I get the reverted version.

You 've been amazingly helpful, and I can't really ask you for more. I'm tempted to mark this as the correct answer, even if it doesn't work fo me.

If I try to put a link to the github repository for the project, my post gets held in a "waiting for moderation" queue.

I think it is safe to say that 99% of the time that developers have tried to use setNeedsDisplay to update a view, it doesn't work. I think you are going down a rabbit hole here. You simply can't beat it into submission.

setNeedsDisplay only marks the view for redrawing. It isn't going to update the underlying data being displayed. You still have to do that. Notifications can be useful to get a signal to just redraw everything. But you are still in a chicken-and-egg situation. setNeedsDisplay, and redrawing are appropriate only if you have overriden one or more "draw" methods. That is the mechanism you would use to trigger your draw methods. But if you are doing something at a higher level, it isn't going to work.

Just how are you getting data into your UI anyway? Are you directly stuffing values into UI objects like NSLabel, NSImage, NSTextField, etc.? If so, you'll have to re-stuff the new values to update the UI. Using bindings is a way to have the UI do that stuffing for you automatically. Bindings will also handle extracting the final values after your UI interaction is complete.

You can post URLs here as long as you remember to remove the scheme. Just post something like "github.com/etresoft/ESHelp". Anybody on this site should be able to figure that out. I think I've been to your github before. I'll see if I can find it.

OK. I found your github project. I don't know how to use the app though. I don't see any way to modify a PDF document to test saving and reverting.

Right off the bat, you are going to be in for a ride. You are trying to write a version of Preview that does not use modern NSDocument behaviour. That is a huge plot point. Virtually all advice you are likely to get regarding NSDocument is going to be telling you how to get modern NSDocument behaviour working properly. Getting NSDocument to work properly can be a challenge. Trying to get NSDocument to do something that it was not designed to do is going to be much more difficult.

That detail aside, I can tell you what the problem is. You are initializing your PDF view in viewDidAppear. That isn't getting called as a side effect of revert. Therefore, the PDF view is still displaying the old PDF document. You may have reset the PDFDocument in your document object, but the reference to the PDF document that the PDF view holds is still the old one.

I think part of the problem may be the iOS-style ViewController logic you are using. I think I had decided against mentioning this aspect in an effort to avoid discouragement. But I guess I should say something. This architecture is something that Apple ported to the Mac as a way to make iOS developers feel more comfortable. Apple is slowly deprecating all of macOS, regardless of what any Apple employee may say about the matter. iOS doesn't have bindings, so the UI flow is completely different there. There are lots of attractive-looking hooks like viewDidAppear, but you have to be careful with them. It does what it says it says. But It probably doesn't get called after revert. Since it is part of a delicate architecture much like NSDocument, you shouldn't force the issue by trying to call it manually. I can't say for sure when viewDidAppear is or is not called. I don't use the iOS ViewController style logic myself. But I generally avoid making any such assumptions about what Apple's code might do.

You may be able to just assign a new document to the view. The notifications listed in PDFView seem to suggest that will work. I think the notification might be the right approach here. When you get a notification that your underlying data has changed, just refresh your PDF view by changing its document. You can have your viewDidAppear method call the same update method that your notification handler calls.

John, Many thanks for this. (You should be able to Rotate and Delete pages in the app.)

Can you clarify "You are initializing your PDF view in viewDidAppear. That isn't getting called as a side effect of revert."?

I tried moving everything from viewDidAppear to viewDidLoad, but I just got a grey window.

I've no objection to using NSDocument 'the right way', I'm just using it as minimally as I can to get it working. As the app grows, no doubt that will change. I've read Apple's pages on NSDocument, which describes a text doc, and I can't always see how it applies.

I know it's a "long-term project": but I've been filing BRs and ERs about Preview for 10 years, so... 😁

  // OVERRIDES
    override func viewDidLoad() {
        super.viewDidLoad()
        NotificationCenter.default.addObserver(self, selector: #selector(handleDocumentReverted), name: .documentReverted, object: nil)
        // Do any additional setup after loading the view.
    }
   
    override func viewDidAppear() {
       
        let theThumbnailSize = CGSize(width: 200, height: 200)
        self.thePDFView?.document = document?.thePDFDocument
        // self.thePDFView?.autoScales = true
        self.thePDFView?.displayMode = theDisplayMode
        self.thePDFView?.displaysAsBook = bookState

        self.theThumbnailView?.pdfView = self.thePDFView
        self.theThumbnailView?.thumbnailSize = theThumbnailSize
    }
   

    override var representedObject: Any? {
        didSet {
        // Update the view, if already loaded.
        }
    }

hi,

after reading John's reply and then your code above, i'm sorry now to say that the advice i was offering was only correct if you were following MVC/iOS paradigm. i assumed you were. my apologies for not fully finishing the code.

John is right on three fronts:

(1) MacOS is becoming more iOS-like; the notions of an NSViewController even being in the responder chain and having viewWillAppear() and viewDidAppear() were only introduced maybe 4 years ago.

(2) your code is setting up its views to have direct references to the things they draw (along with some parameters). when the NSDocument is reverted, you're not telling the views that their references must be updated first before the views are redrawn.

(3) you need to set up references for the NSViews outside of viewDidAppear(). in fact, i don't see that you even want to touch viewDidAppear(). then you can call that code both upon startup and when revert happens.

so, based on your question and seeing your most recent code above, John's asking you to rewrite this way:

override func viewDidLoad() { 
  super.viewDidLoad() 
  NotificationCenter.default.addObserver(self, selector: #selector(handleDocumentReverted), name: .documentReverted, object: nil) 
  loadViewParameters() 
} 


func loadViewParameters() { 
  let theThumbnailSize = CGSize(width: 200, height: 200) 
  self.thePDFView?.document = document?.thePDFDocument 
  // self.thePDFView?.autoScales = true 
  self.thePDFView?.displayMode = theDisplayMode 
  self.thePDFView?.displaysAsBook = bookState 


  self.theThumbnailView?.pdfView = self.thePDFView 
  self.theThumbnailView?.thumbnailSize = theThumbnailSize 
} 


@objc func handleDocumentReverted(_ note: Notification) { 
  // i mentioned that you may not have to check directly in some cases
  // whether you're responding to the right document's revert in these 2 lines:
  // let sendingDocument = note.object as? Document 
  // guard sendingDocument == document else { return }
  loadViewParameters()
  thePDFView.setNeedsDisplay(thePDFView.bounds) 
  theThumbnailView.setNeedsDisplay(theThumbnailView.bounds) 
}

hope that helps,

DMG

DMG, you have nothing to apologise for! If there's any fault, it's in my not supplying enough relevant code to start with. (Though, of course, knowing what's relevant is half the battle.)

Your code worked with one alteration: As is, I got a blank grey window at launch. By including loadViewParameters in viewWillAppear() as well as in the Revert Handler, it works perfectly! Nothing was needed in viewDidLoad except the Notification. (In fact, the super isn't even necessary -- according to Apple, the default implementation does nothing.)

So I'm going to mark it as the correct answer.

Right, onto Undo! 😕

DMG's reply is very nice. I think the viewDidLoad() method was used just because you mentioned trying that most recently. One of the most tricky things to deal with is lifecycle. Consider these events:

app init

open doc

create doc

init doc

read nib

read data

layout views

display views

redraw views

modify data

modify views

revert data

revert views

save data

close window

close doc

dealloc doc

quit app

This list is incomplete. There are different overrides for each of these events. Sometimes there are multiple potential overrides you could do. These events are not necessarily in the proper order. They can sometimes occur in different orders depending on the circumstances. And yet, certain events must always come before certain other events. Sometimes, certain events don't occur. For example, if you quit the app without closing documents first, the document dealloc is never called, so be careful what you do in there.

At least undo is much easier and more pleasant to work with. I had one app with extensive custom undo support. I forget which app it was. Ah, I remember! I was stupid enough to write an RSS reader once. You can use invocations to record undo operations. For each operation, identify an operation that would undo it. Then, do something like this:

[[undoManager prepareWithInvocationTarget: self]

markArticles: articles read: !read];

The "markArticles:read:" method is one of mine. When the user performs undo, it just calls this method with the given parameters. In your case, if the user is performing a rotate right 90deg, you issue a prepareWithInvocationTarget that does a rotate left 90deg.

I'm not sure how Swift will handle that. Maybe it uses blocks now. The date on my source file is 2012. 🙂