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.

Sign up for GitHub

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

Jump to bottom

DEFEDIT-7152 Added preference to disable loading of external changes on app focus #7158

Merged
merged 2 commits into from
Nov 21, 2022
Merged

DEFEDIT-7152 Added preference to disable loading of external changes on app focus #7158

merged 2 commits into from
Nov 21, 2022

Conversation

matgis
Copy link
Contributor

@matgis matgis commented Nov 17, 2022 •
edited by britzl
Loading

Fixes #7152.

User-facing changes

  • Added checkbox Load External Changes on App Focus to the General Preferences page. When disabled, the editor won't scan for external changes when it receives focus. A new menu entry Load External Changes will appear in the File menu so users can trigger the process manually.
  • Moved Code Editor Font setting from the General Preferences page to the Code Preferences page.

matgis
matgis commented Nov 17, 2022
View reviewed changes
editor/src/clj/editor/app_view.clj
(prefs-dialog/open-prefs prefs)
(workspace/update-build-settings! workspace prefs)))
(workspace/update-build-settings! workspace prefs)
(ui/invalidate-menubar-item! ::file)))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Disabling auto-load of external changes activates the Load External Changes menu item in the File menu. When auto-load of external changes is enabled, the menu item will not be visible.

editor/src/clj/editor/app_view.clj
(when (and (async-reload-on-app-focus? prefs)
(can-async-reload?))
(async-reload! app-view changes-view workspace [])))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved to app-view from boot-open-project.

editor/src/clj/editor/app_view.clj

(defn build-in-progress? []
@build-in-progress-atom)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved up, since we need it above.

editor/src/clj/editor/boot_open_project.clj
(not (sync/sync-dialog-open?))
(disk-availability/available?))
(async-reload! workspace changes-view)))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved to app-view.

editor/src/clj/editor/boot_open_project.clj
(.lookup root "#changes-container"))
changes-view (changes-view/make-changes-view *view-graph* workspace prefs (.lookup root "#changes-container")
(fn [changes-view moved-files]
(app-view/async-reload! app-view changes-view workspace moved-files)))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed async-reload! callback signature to only provide the changes-view and the moved-files. This is called when files are reverted from the Changes View.

Changed argument order to make-changes-view, putting the async-reload! callback in the last argument position.

editor/src/clj/editor/changes_view.clj
@@ -179,7 +179,7 @@
(when (some? git)
(.close git)))))

(defn make-changes-view [view-graph workspace prefs async-reload! ^Parent parent]
(defn make-changes-view [view-graph workspace prefs ^Parent parent async-reload!]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed argument order to make-changes-view, putting the async-reload! callback in the last argument position.

editor/src/clj/editor/keymap.clj
@@ -30,127 +30,129 @@
;; of mistake.
;; Note that specifying Shortcut key (which is Meta on mac and Ctrl on windows
;; and linux) is not allowed.
{:macos [["A" :add]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed indentation for the :macos block (due to earlier rename from :darwin), but also added ["Alt+Meta+Y" :async-reload]. This key combination used to perform the same operation in IntelliJ (not sure they have manual refresh anymore?) and is suitably unique for our needs.

editor/src/clj/editor/prefs_dialog.clj
@@ -72,7 +72,7 @@
control))

(defn- create-prefs-row! [prefs ^GridPane grid row desc]
(let [label (Label. (str (:label desc) ":"))
(let [label (Label. (:label desc))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed colon-suffix for Preference Dialog labels on a whim.

editor/src/clj/editor/prefs_dialog.clj
{:name "Code"
:prefs [{:label "Custom Editor" :type :string :key "code-custom-editor" :default ""}
{:label "Open File" :type :string :key "code-open-file" :default "{file}"}
{:label "Open File at Line" :type :string :key "code-open-file-at-line" :default "{file}:{line}"}]}
{:label "Open File at Line" :type :string :key "code-open-file-at-line" :default "{file}:{line}"}
{:label "Code Editor Font (Requires Restart)" :type :string :key "code-editor-font-name" :default "Dejavu Sans Mono"}]}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed capitalization, added Load External Changes on App Focus, and moved Code Editor Font to the Code page.

editor/src/clj/editor/ui.clj
@@ -1500,7 +1500,7 @@
(when-some [parent (or (.getParentMenu old) menu-bar)]
(when-some [parent-children (menu-items parent)]
(let [index (.indexOf parent-children old)]
(when (pos? index)
(when-not (neg? index)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Bugfix: (ui/invalidate-menubar-item! ::file) wouldn't do anything since it is at index zero.

@matgis matgis requested a review from vlaaad November 17, 2022 16:54
@matgis matgis force-pushed the DEFEDIT-7152-load-external-changes-manually branch from 2f8dda2 to 60a599f Compare November 17, 2022 17:06
matgis
matgis commented Nov 17, 2022
View reviewed changes
editor/src/clj/editor/keymap.clj Outdated
"W" ; :move-tool
"Ctrl+Alt+Y" ; :async-reload
"Shift+A" ; :add-secondary
"Shift+E"}) ; :erase-tool
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mostly whitespace, but added "Ctrl+Alt+Y" here. It seems Ctrl+Alt+[Anything] is considered "typeable" under Windows, because the AltGr key is the same as pressing Ctrl+Alt. AltGr+Y appears to produce an ü on most keyboard layouts. I don't know, maybe we should choose a different shortcut?

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe use Ctrl+Y on Windows?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On second thought, I think I'll just revert this and change it to use Shift+Cmd/Ctrl+Y on all platforms.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good!

vlaaad
vlaaad approved these changes Nov 21, 2022
View reviewed changes
Copy link
Contributor

@vlaaad vlaaad left a comment

Choose a reason for hiding this comment

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

Ship it! 👍

editor/src/clj/editor/keymap.clj Outdated
"W" ; :move-tool
"Ctrl+Alt+Y" ; :async-reload
"Shift+A" ; :add-secondary
"Shift+E"}) ; :erase-tool
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe use Ctrl+Y on Windows?

@matgis matgis merged commit fc85b06 into editor-dev Nov 21, 2022
@matgis matgis deleted the DEFEDIT-7152-load-external-changes-manually branch November 21, 2022 13:44
vlaaad pushed a commit that referenced this pull request Nov 23, 2022
@matgis @vlaaad
DEFEDIT-7152 Added preference to disable loading of external changes …
07840e2 
…on app focus (#7158)

* DEFEDIT-7152 Added preference to disable loading of external changes on app focus

* Review fix
@britzl britzl linked an issue Nov 25, 2022 that may be closed by this pull request
Option for editor do "Loading external changes" manually, by hotkey #7152
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vlaaad vlaaad vlaaad approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Option for editor do "Loading external changes" manually, by hotkey
2 participants
@matgis @vlaaad