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

Tail feature #486

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Sources/XiEditor/Client.swift
Original file line number Diff line number Diff line change
Expand Up @@ -86,4 +86,7 @@ protocol XiClient: AnyObject {

/// A notification containing the current replace status.
func replaceStatus(viewIdentifier: String, status: ReplaceStatus)

/// A notification telling whether tail was enabled/disabled.
func enableTailing(viewIdentifier: String, isTailEnabled: Bool)
}
8 changes: 8 additions & 0 deletions Sources/XiEditor/ClientImplementation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,14 @@ class ClientImplementation: XiClient, DocumentsProviding, ConfigCacheProviding,
}
}
}

func enableTailing(viewIdentifier: String, isTailEnabled: Bool) {
let document = documentForViewIdentifier(viewIdentifier: viewIdentifier)
DispatchQueue.main.async {
document?.editViewController?.enableTailing(isTailEnabled)
}
}


// Stores the config dict so new windows don't have to wait for core to send it.
// The main purpose of this is ensuring that `unified_titlebar` applies immediately.
Expand Down
6 changes: 6 additions & 0 deletions Sources/XiEditor/Core/CoreNotification.swift
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ enum CoreNotification {

case findStatus(viewIdentifier: ViewIdentifier, status: [FindStatus])
case replaceStatus(viewIdentifier: ViewIdentifier, status: ReplaceStatus)
case enableTailing(viewIdentifier: ViewIdentifier, isTailEnabled: Bool)

static func fromJson(_ json: [String: Any]) -> CoreNotification? {
guard
Expand Down Expand Up @@ -228,6 +229,11 @@ enum CoreNotification {
{
return .replaceStatus(viewIdentifier: viewIdentifier!, status: replaceStatus)
}
case "enable_tailing":
if let isTailEnabled = jsonParams["is_tail_enabled"] as? Bool
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think enable_tailing should be toggle_tailing instead, since it's way clearer in conveying what it does exactly. enable_tailing implies that it always enables tailing for any file, not what it actually does (which is toggle the tailing state for a specific view).

Since you changed this here, your plugin doesn't handle the old JSON properly. Perhaps you should update your other PR as well with this change?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I havent push the change. I'll do it based on outcome of the naming discussion we are having.

{
return .enableTailing(viewIdentifier: viewIdentifier!, isTailEnabled: isTailEnabled)
}

default:
assertionFailure("Unsupported notification method from core: \(jsonMethod)")
Expand Down
37 changes: 37 additions & 0 deletions Sources/XiEditor/EditViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,11 @@ class EditViewController: NSViewController, EditViewDataSource, FindDelegate, Sc
updateLanguageMenu()
}
}

/// Flag to determine if current file is being tailed.
var isTailEnabled: Bool = false

var isExistingFile: Bool = false

// used to calculate the gutter width. Initial -1 so that a new document
// still triggers update of gutter width.
Expand Down Expand Up @@ -329,6 +334,12 @@ class EditViewController: NSViewController, EditViewDataSource, FindDelegate, Sc
_previousViewportSize = CGSize.zero
redrawEverything()
}

func enableTailMenu() {
let toggleTailSubMenu = NSApplication.shared.mainMenu!.item(withTitle: "Debug")!.submenu!.item(withTitle: "Tail File")
toggleTailSubMenu!.isEnabled = true
self.isExistingFile = true
}

/// If font size or theme changes, we invalidate all views.
func redrawEverything() {
Expand Down Expand Up @@ -711,6 +722,10 @@ class EditViewController: NSViewController, EditViewDataSource, FindDelegate, Sc
document.xiCore.setTheme(themeName: sender.title)
}

@IBAction func debugToggleTail(_ sender: NSMenuItem) {
document.xiCore.toggleTailConfig(identifier: document.coreViewIdentifier!, enabled: !self.isTailEnabled)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could probably rewrite this in a way so we don't have to do !self.isTailEnabled here, since it feels counter intuitive.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rewrite how?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just a little nit, but I'd prefer if this was

document.xiCore.toggleTailConfig(identifier: document.coreViewIdentifier!, enabled: self.isTailEnabled)

}

@IBAction func debugSetLanguage(_ sender: NSMenuItem) {
guard sender.state != NSControl.StateValue.on else { print("language already active"); return }
document.xiCore.setLanguage(identifier: document.coreViewIdentifier!, languageName: sender.title)
Expand Down Expand Up @@ -840,11 +855,28 @@ class EditViewController: NSViewController, EditViewDataSource, FindDelegate, Sc
item.state = findViewController.showMultipleSearchQueries ? .on : .off
}

func updateTailMenu() {
let toggleTailSubMenu = NSApplication.shared.mainMenu!.item(withTitle: "Debug")!.submenu!.item(withTitle: "Tail File")
if self.isExistingFile {
toggleTailSubMenu!.isEnabled = true
if self.isTailEnabled {
toggleTailSubMenu!.state = NSControl.StateValue.on
} else {
toggleTailSubMenu!.state = NSControl.StateValue.off
}
} else {
toggleTailSubMenu!.isEnabled = false
toggleTailSubMenu!.state = NSControl.StateValue.off
self.isTailEnabled = false
}
}

// Gets called when active window changes
func updateMenuState() {
updatePluginMenu()
updateLanguageMenu()
updateFindMenu()
updateTailMenu()
}

@objc func handleCommand(_ sender: NSMenuItem) {
Expand Down Expand Up @@ -919,6 +951,11 @@ class EditViewController: NSViewController, EditViewDataSource, FindDelegate, Sc
languagesMenu.addItem(item)
}
}

public func enableTailing(_ isTailEnabled: Bool) {
self.isTailEnabled = isTailEnabled
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.. and this part is instead:
self.isTailEnabled = !isTailEnabled

This avoids having to rely on state from another method.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my long comment below.

self.updateTailMenu()
}

@IBAction func gotoLine(_ sender: AnyObject) {
guard let window = self.view.window else { return }
Expand Down
9 changes: 8 additions & 1 deletion Sources/XiEditor/Main.storyboard
Original file line number Diff line number Diff line change
Expand Up @@ -1111,7 +1111,7 @@ Gw
</menuItem>
<menuItem title="Debug" id="z0D-vd-GVg" userLabel="Debug">
<modifierMask key="keyEquivalentModifierMask"/>
<menu key="submenu" title="Debug" id="WuB-lA-TEF">
<menu key="submenu" title="Debug" autoenablesItems="NO" id="WuB-lA-TEF">
<items>
<menuItem title="Print Selection Spans" keyEquivalent="" id="8QK-XZ-5IS">
<modifierMask key="keyEquivalentModifierMask"/>
Expand Down Expand Up @@ -1209,6 +1209,12 @@ Gw
<action selector="openErrorLogFolder:" target="azf-ih-6fI" id="dlx-rd-R59"/>
</connections>
</menuItem>
<menuItem title="Tail File" enabled="NO" toolTip="Start tailing current file" id="ERX-yh-iVz">
<modifierMask key="keyEquivalentModifierMask"/>
<connections>
<action selector="debugToggleTail:" target="7er-QZ-amI" id="Nk3-gD-C6i"/>
</connections>
</menuItem>
</items>
</menu>
</menuItem>
Expand Down Expand Up @@ -1238,6 +1244,7 @@ Gw
</connections>
</customObject>
<userDefaultsController id="yco-8S-n0A"/>
<userDefaultsController id="auS-BU-5TV"/>
</objects>
<point key="canvasLocation" x="-295" y="-593"/>
</scene>
Expand Down
2 changes: 2 additions & 0 deletions Sources/XiEditor/RPCSending.swift
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,8 @@ class StdoutRPCSender: RPCSending {
self.client?.findStatus(viewIdentifier: viewIdentifier, status: status)
case let .replaceStatus(viewIdentifier, status):
self.client?.replaceStatus(viewIdentifier: viewIdentifier, status: status)
case let .enableTailing(viewIdentifier, isTailEnabled):
self.client?.enableTailing(viewIdentifier: viewIdentifier, isTailEnabled: isTailEnabled)
}
}

Expand Down
7 changes: 7 additions & 0 deletions Sources/XiEditor/XiCore.swift
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ protocol XiCore: class {
/// `Document` calls are migrated to it's own protocol.
func sendRpcAsync(_ method: String, params: Any, callback: RpcCallback?)
func sendRpc(_ method: String, params: Any) -> RpcResult
/// Will tail opened file if enabled.
/// If toggle succeeds the client will receive a `toggle_tail_config_changed` notification.
func toggleTailConfig(identifier: ViewIdentifier, enabled: Bool)
}

final class CoreConnection: XiCore {
Expand Down Expand Up @@ -109,6 +112,10 @@ final class CoreConnection: XiCore {
let params = ["destination": destination, "frontend_samples": frontendSamples] as AnyObject
sendRpcAsync("save_trace", params: params)
}

func toggleTailConfig(identifier: ViewIdentifier, enabled: Bool) {
sendRpcAsync("toggle_tail", params: ["view_id": identifier, "enabled": enabled])
}

func sendRpcAsync(_ method: String, params: Any, callback: RpcCallback? = nil) {
rpcSender.sendRpcAsync(method, params: params, callback: callback)
Expand Down
5 changes: 5 additions & 0 deletions Sources/XiEditor/XiDocumentController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ class XiDocumentController: NSDocumentController, AlertPresenting {
currentDocument.fileURL = url
self.setIdentifier(result, forDocument: currentDocument)
currentDocument.editViewController!.prepareForReuse()
currentDocument.editViewController!.enableTailMenu()
completionHandler(currentDocument, false, nil)

case .error(let error):
Expand Down Expand Up @@ -182,6 +183,10 @@ class XiDocumentController: NSDocumentController, AlertPresenting {
let viewIdentifier = result as! String
self.setIdentifier(viewIdentifier, forDocument: document)
document.coreViewIdentifier = viewIdentifier
if url != nil {
document.editViewController?.isExistingFile = true
}
document.editViewController?.updateTailMenu()
case .error(let error):
document.close()
self.showAlert(with: error.message)
Expand Down
3 changes: 3 additions & 0 deletions Tests/IntegrationTests/TestClientImplementation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -86,4 +86,7 @@ class TestClientImplementation: XiClient {

func replaceStatus(viewIdentifier: String, status: ReplaceStatus) {
}

func enableTailing(viewIdentifier: String, isTailEnabled: Bool) {
}
}