From c567d7d043b689d72eabbd671eab8ae8805dadc1 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Wed, 3 Jun 2015 11:23:29 +0100 Subject: Fix marks (incl. global marks)... Fixes #1712: - Make global marks work. - Add mode indicator. - Don't fail for global marks on background page if mark is not set. - Give HUD warning for global marks if global mark is not set. (The diff is big but, which the exception of infrastructure refactoring, the main change is to not exit on , thereby fixing #1712). --- content_scripts/marks.coffee | 101 +++++++++++++++++++++++++------------------ 1 file changed, 60 insertions(+), 41 deletions(-) (limited to 'content_scripts/marks.coffee') diff --git a/content_scripts/marks.coffee b/content_scripts/marks.coffee index 316ab951..f3dfd465 100644 --- a/content_scripts/marks.coffee +++ b/content_scripts/marks.coffee @@ -1,45 +1,64 @@ -root = window.Marks = {} -root.activateCreateMode = -> - handlerStack.push keydown: (e) -> - keyChar = KeyboardUtils.getKeyChar(event) - return unless keyChar isnt "" +exit = (mode, continuation = null) -> + mode.exit() + continuation?() + false - if /[A-Z]/.test keyChar - chrome.runtime.sendMessage { - handler: 'createMark', - markName: keyChar - scrollX: window.scrollX, - scrollY: window.scrollY - }, -> HUD.showForDuration "Created global mark '#{keyChar}'", 1000 - else if /[a-z]/.test keyChar - [baseLocation, sep, hash] = window.location.href.split '#' - localStorage["vimiumMark|#{baseLocation}|#{keyChar}"] = JSON.stringify - scrollX: window.scrollX, - scrollY: window.scrollY - HUD.showForDuration "Created local mark '#{keyChar}'", 1000 +Marks = + activateCreateMode: -> + mode = new Mode + name: "create-mark" + indicator: "Create mark?" + keypress: -> false + keyup: -> false + keydown: (event) -> + keyChar = KeyboardUtils.getKeyChar(event) + if /[A-Z]/.test keyChar + exit mode, -> + chrome.runtime.sendMessage + handler: 'createMark' + markName: keyChar + scrollX: window.scrollX + scrollY: window.scrollY + , -> HUD.showForDuration "Created global mark '#{keyChar}'.", 1000 + else if /[a-z]/.test keyChar + [baseLocation, sep, hash] = window.location.href.split '#' + localStorage["vimiumMark|#{baseLocation}|#{keyChar}"] = JSON.stringify + scrollX: window.scrollX, + scrollY: window.scrollY + exit mode, -> HUD.showForDuration "Created local mark '#{keyChar}'.", 1000 + else if event.shiftKey + false + else + exit mode - @remove() + activateGotoMode: -> + mode = new Mode + name: "goto-mark" + indicator: "Go to mark?" + keypress: -> false + keyup: -> false + keydown: (event) -> + keyChar = KeyboardUtils.getKeyChar(event) + if /[A-Z]/.test keyChar + exit mode, -> + chrome.runtime.sendMessage + handler: 'gotoMark' + markName: keyChar + else if /[a-z]/.test keyChar + [baseLocation, sep, hash] = window.location.href.split '#' + markString = localStorage["vimiumMark|#{baseLocation}|#{keyChar}"] + exit mode, -> + if markString? + mark = JSON.parse markString + window.scrollTo mark.scrollX, mark.scrollY + HUD.showForDuration "Jumped to local mark '#{keyChar}'", 1000 + else + HUD.showForDuration "Local mark not set: '#{keyChar}'.", 1000 + else if event.shiftKey + false + else + exit mode - false - -root.activateGotoMode = -> - handlerStack.push keydown: (e) -> - keyChar = KeyboardUtils.getKeyChar(event) - return unless keyChar isnt "" - - if /[A-Z]/.test keyChar - chrome.runtime.sendMessage - handler: 'gotoMark' - markName: keyChar - else if /[a-z]/.test keyChar - [baseLocation, sep, hash] = window.location.href.split '#' - markString = localStorage["vimiumMark|#{baseLocation}|#{keyChar}"] - if markString? - mark = JSON.parse markString - window.scrollTo mark.scrollX, mark.scrollY - HUD.showForDuration "Jumped to local mark '#{keyChar}'", 1000 - - @remove() - - false +root = exports ? window +root.Marks = Marks -- cgit v1.2.3 From 03ef7f2a8525e8fbfc67b04d7a0ce47522449d03 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Wed, 3 Jun 2015 11:42:25 +0100 Subject: Refactor to avoid having to cover all keyboard event cases. It's pretty common that we want to suppress all keyboard events, so let's support that in modes.coffee, thereby simplifying handlers elsewhere. --- content_scripts/marks.coffee | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) (limited to 'content_scripts/marks.coffee') diff --git a/content_scripts/marks.coffee b/content_scripts/marks.coffee index f3dfd465..8ba45fd4 100644 --- a/content_scripts/marks.coffee +++ b/content_scripts/marks.coffee @@ -2,15 +2,13 @@ exit = (mode, continuation = null) -> mode.exit() continuation?() - false Marks = activateCreateMode: -> mode = new Mode name: "create-mark" indicator: "Create mark?" - keypress: -> false - keyup: -> false + suppressAllKeyboardEvents: true keydown: (event) -> keyChar = KeyboardUtils.getKeyChar(event) if /[A-Z]/.test keyChar @@ -27,17 +25,14 @@ Marks = scrollX: window.scrollX, scrollY: window.scrollY exit mode, -> HUD.showForDuration "Created local mark '#{keyChar}'.", 1000 - else if event.shiftKey - false - else + else if not event.shiftKey exit mode activateGotoMode: -> mode = new Mode name: "goto-mark" indicator: "Go to mark?" - keypress: -> false - keyup: -> false + suppressAllKeyboardEvents: true keydown: (event) -> keyChar = KeyboardUtils.getKeyChar(event) if /[A-Z]/.test keyChar @@ -55,9 +50,7 @@ Marks = HUD.showForDuration "Jumped to local mark '#{keyChar}'", 1000 else HUD.showForDuration "Local mark not set: '#{keyChar}'.", 1000 - else if event.shiftKey - false - else + else if not event.shiftKey exit mode root = exports ? window -- cgit v1.2.3 From 291e7fd67de9e1c4bd0bc5048ab7344424f19b30 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Wed, 3 Jun 2015 15:27:45 +0100 Subject: Re-implement Marks, incl `` binding. --- content_scripts/marks.coffee | 94 +++++++++++++++++++++++++++----------------- 1 file changed, 57 insertions(+), 37 deletions(-) (limited to 'content_scripts/marks.coffee') diff --git a/content_scripts/marks.coffee b/content_scripts/marks.coffee index 8ba45fd4..fab509a7 100644 --- a/content_scripts/marks.coffee +++ b/content_scripts/marks.coffee @@ -1,57 +1,77 @@ -exit = (mode, continuation = null) -> - mode.exit() - continuation?() - Marks = + mode: null + previousPosition: null + + exit: (continuation = null) -> + @mode?.exit() + @mode = null + continuation?() + + # This returns the key which is used for storing mark locations in localStorage. + getLocationKey: (keyChar) -> + "vimiumMark|#{window.location.href.split('#')[0]}|#{keyChar}" + + showMessage: (message, keyChar) -> + HUD.showForDuration "#{message} \"#{keyChar}\".", 1000 + activateCreateMode: -> - mode = new Mode + @mode = new Mode name: "create-mark" - indicator: "Create mark?" + indicator: "Create mark..." suppressAllKeyboardEvents: true - keydown: (event) -> - keyChar = KeyboardUtils.getKeyChar(event) - if /[A-Z]/.test keyChar - exit mode, -> + keypress: (event) => + keyChar = String.fromCharCode event.charCode + # If is depressed, then it's a global mark, otherwise it's a local mark. This is consistent + # vim's [A-Z] for global marks, [a-z] for local marks. However, it also admits other non-Latin + # characters. + if event.shiftKey + @exit => chrome.runtime.sendMessage handler: 'createMark' markName: keyChar scrollX: window.scrollX scrollY: window.scrollY - , -> HUD.showForDuration "Created global mark '#{keyChar}'.", 1000 - else if /[a-z]/.test keyChar - [baseLocation, sep, hash] = window.location.href.split '#' - localStorage["vimiumMark|#{baseLocation}|#{keyChar}"] = JSON.stringify - scrollX: window.scrollX, - scrollY: window.scrollY - exit mode, -> HUD.showForDuration "Created local mark '#{keyChar}'.", 1000 - else if not event.shiftKey - exit mode - - activateGotoMode: -> - mode = new Mode + , => @showMessage "Created global mark", keyChar + else + @exit => @markPosition keyChar + + markPosition: (keyChar = null) -> + markString = JSON.stringify scrollX: window.scrollX, scrollY: window.scrollY + if keyChar? + localStorage[@getLocationKey keyChar] = markString + @showMessage "Created local mark", keyChar + else + @previousPosition = markString + + activateGotoMode: (registryEntry) -> + # We pick off the last character of the key sequence used to launch this command. Usually this is just "`". + # We then use that character, so together usually the sequence "``", to jump back to the previous + # position. The "previous position" is recorded below, and is registered via @markPosition() elsewhere + # for various other jump-like commands. + previousPositionKey = registryEntry.key[registryEntry.key.length-1..] + @mode = new Mode name: "goto-mark" - indicator: "Go to mark?" + indicator: "Go to mark..." suppressAllKeyboardEvents: true - keydown: (event) -> - keyChar = KeyboardUtils.getKeyChar(event) - if /[A-Z]/.test keyChar - exit mode, -> + keypress: (event) => + keyChar = String.fromCharCode event.charCode + if event.shiftKey + @exit -> chrome.runtime.sendMessage handler: 'gotoMark' markName: keyChar - else if /[a-z]/.test keyChar - [baseLocation, sep, hash] = window.location.href.split '#' - markString = localStorage["vimiumMark|#{baseLocation}|#{keyChar}"] - exit mode, -> + else + markString = + if keyChar == previousPositionKey then @previousPosition else localStorage[@getLocationKey keyChar] + @exit => if markString? - mark = JSON.parse markString - window.scrollTo mark.scrollX, mark.scrollY - HUD.showForDuration "Jumped to local mark '#{keyChar}'", 1000 + @markPosition() + position = JSON.parse markString + window.scrollTo position.scrollX, position.scrollY + @showMessage "Jumped to local mark", keyChar else - HUD.showForDuration "Local mark not set: '#{keyChar}'.", 1000 - else if not event.shiftKey - exit mode + @showMessage "Local mark not set", keyChar root = exports ? window root.Marks = Marks -- cgit v1.2.3 From e005d3e5b7a06a949138109b2c57555f9a5db2b9 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Wed, 3 Jun 2015 18:39:03 +0100 Subject: Refactor (to setPreviousPosition) to clarify Marks logic. --- content_scripts/marks.coffee | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) (limited to 'content_scripts/marks.coffee') diff --git a/content_scripts/marks.coffee b/content_scripts/marks.coffee index fab509a7..9c6b1458 100644 --- a/content_scripts/marks.coffee +++ b/content_scripts/marks.coffee @@ -12,6 +12,12 @@ Marks = getLocationKey: (keyChar) -> "vimiumMark|#{window.location.href.split('#')[0]}|#{keyChar}" + getMarkString: -> + JSON.stringify scrollX: window.scrollX, scrollY: window.scrollY + + setPreviousPosition: -> + @previousPosition = @getMarkString() + showMessage: (message, keyChar) -> HUD.showForDuration "#{message} \"#{keyChar}\".", 1000 @@ -34,21 +40,16 @@ Marks = scrollY: window.scrollY , => @showMessage "Created global mark", keyChar else - @exit => @markPosition keyChar - - markPosition: (keyChar = null) -> - markString = JSON.stringify scrollX: window.scrollX, scrollY: window.scrollY - if keyChar? - localStorage[@getLocationKey keyChar] = markString - @showMessage "Created local mark", keyChar - else - @previousPosition = markString + @exit => + markString = JSON.stringify scrollX: window.scrollX, scrollY: window.scrollY + localStorage[@getLocationKey keyChar] = @getMarkString() + @showMessage "Created local mark", keyChar activateGotoMode: (registryEntry) -> # We pick off the last character of the key sequence used to launch this command. Usually this is just "`". # We then use that character, so together usually the sequence "``", to jump back to the previous - # position. The "previous position" is recorded below, and is registered via @markPosition() elsewhere - # for various other jump-like commands. + # position. The "previous position" is recorded below, and is registered via @setPreviousPosition() + # elsewhere for various other jump-like commands. previousPositionKey = registryEntry.key[registryEntry.key.length-1..] @mode = new Mode name: "goto-mark" @@ -66,7 +67,7 @@ Marks = if keyChar == previousPositionKey then @previousPosition else localStorage[@getLocationKey keyChar] @exit => if markString? - @markPosition() + @setPreviousPosition() position = JSON.parse markString window.scrollTo position.scrollX, position.scrollY @showMessage "Jumped to local mark", keyChar -- cgit v1.2.3 From 6720f4c5a511e5e7b39e6a59e989353f7fb09792 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Wed, 3 Jun 2015 18:57:39 +0100 Subject: Add mark commands to README, simple refactoring. --- content_scripts/marks.coffee | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) (limited to 'content_scripts/marks.coffee') diff --git a/content_scripts/marks.coffee b/content_scripts/marks.coffee index 9c6b1458..73c019da 100644 --- a/content_scripts/marks.coffee +++ b/content_scripts/marks.coffee @@ -31,19 +31,17 @@ Marks = # If is depressed, then it's a global mark, otherwise it's a local mark. This is consistent # vim's [A-Z] for global marks, [a-z] for local marks. However, it also admits other non-Latin # characters. - if event.shiftKey - @exit => - chrome.runtime.sendMessage - handler: 'createMark' - markName: keyChar - scrollX: window.scrollX - scrollY: window.scrollY - , => @showMessage "Created global mark", keyChar - else - @exit => - markString = JSON.stringify scrollX: window.scrollX, scrollY: window.scrollY - localStorage[@getLocationKey keyChar] = @getMarkString() - @showMessage "Created local mark", keyChar + @exit => + if event.shiftKey + chrome.runtime.sendMessage + handler: 'createMark' + markName: keyChar + scrollX: window.scrollX + scrollY: window.scrollY + , => @showMessage "Created global mark", keyChar + else + localStorage[@getLocationKey keyChar] = @getMarkString() + @showMessage "Created local mark", keyChar activateGotoMode: (registryEntry) -> # We pick off the last character of the key sequence used to launch this command. Usually this is just "`". -- cgit v1.2.3 From 457a976421107f9a67ac43090b0370366427bce1 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Wed, 3 Jun 2015 19:13:17 +0100 Subject: Add mark commands to README, more simple refactoring. --- content_scripts/marks.coffee | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) (limited to 'content_scripts/marks.coffee') diff --git a/content_scripts/marks.coffee b/content_scripts/marks.coffee index 73c019da..971feadf 100644 --- a/content_scripts/marks.coffee +++ b/content_scripts/marks.coffee @@ -54,16 +54,15 @@ Marks = indicator: "Go to mark..." suppressAllKeyboardEvents: true keypress: (event) => - keyChar = String.fromCharCode event.charCode - if event.shiftKey - @exit -> + @exit => + keyChar = String.fromCharCode event.charCode + if event.shiftKey chrome.runtime.sendMessage handler: 'gotoMark' markName: keyChar - else - markString = - if keyChar == previousPositionKey then @previousPosition else localStorage[@getLocationKey keyChar] - @exit => + else + markString = + if keyChar == previousPositionKey then @previousPosition else localStorage[@getLocationKey keyChar] if markString? @setPreviousPosition() position = JSON.parse markString -- cgit v1.2.3 From b356e50677e1ae7ba68e27b1b70e59744e129627 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Thu, 4 Jun 2015 13:37:40 +0100 Subject: Exit marks modes on escape. --- content_scripts/marks.coffee | 2 ++ 1 file changed, 2 insertions(+) (limited to 'content_scripts/marks.coffee') diff --git a/content_scripts/marks.coffee b/content_scripts/marks.coffee index 971feadf..fc30849a 100644 --- a/content_scripts/marks.coffee +++ b/content_scripts/marks.coffee @@ -25,6 +25,7 @@ Marks = @mode = new Mode name: "create-mark" indicator: "Create mark..." + exitOnEscape: true suppressAllKeyboardEvents: true keypress: (event) => keyChar = String.fromCharCode event.charCode @@ -52,6 +53,7 @@ Marks = @mode = new Mode name: "goto-mark" indicator: "Go to mark..." + exitOnEscape: true suppressAllKeyboardEvents: true keypress: (event) => @exit => -- cgit v1.2.3 From 2473475367411fdeb051a6752d0059e337ad09e3 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Thu, 4 Jun 2015 16:23:54 +0100 Subject: Use only ' and ` for jumping to previous position. As suggested by @mrmr1993 in #1716. --- content_scripts/marks.coffee | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) (limited to 'content_scripts/marks.coffee') diff --git a/content_scripts/marks.coffee b/content_scripts/marks.coffee index fc30849a..5bb35f1f 100644 --- a/content_scripts/marks.coffee +++ b/content_scripts/marks.coffee @@ -1,7 +1,8 @@ Marks = + previousPositionRegisters: [ "`", "'" ] + localRegisters: {} mode: null - previousPosition: null exit: (continuation = null) -> @mode?.exit() @@ -16,7 +17,8 @@ Marks = JSON.stringify scrollX: window.scrollX, scrollY: window.scrollY setPreviousPosition: -> - @previousPosition = @getMarkString() + markString = @getMarkString() + @localRegisters[reg] = markString for reg in @previousPositionRegisters showMessage: (message, keyChar) -> HUD.showForDuration "#{message} \"#{keyChar}\".", 1000 @@ -45,11 +47,6 @@ Marks = @showMessage "Created local mark", keyChar activateGotoMode: (registryEntry) -> - # We pick off the last character of the key sequence used to launch this command. Usually this is just "`". - # We then use that character, so together usually the sequence "``", to jump back to the previous - # position. The "previous position" is recorded below, and is registered via @setPreviousPosition() - # elsewhere for various other jump-like commands. - previousPositionKey = registryEntry.key[registryEntry.key.length-1..] @mode = new Mode name: "goto-mark" indicator: "Go to mark..." @@ -63,8 +60,7 @@ Marks = handler: 'gotoMark' markName: keyChar else - markString = - if keyChar == previousPositionKey then @previousPosition else localStorage[@getLocationKey keyChar] + markString = @localRegisters[keyChar] ? localStorage[@getLocationKey keyChar] if markString? @setPreviousPosition() position = JSON.parse markString -- cgit v1.2.3 From a143d2c81a9faaf383a05b0dae2f232db85959a2 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Fri, 5 Jun 2015 06:35:26 +0100 Subject: Global marks; fix indentation. --- content_scripts/marks.coffee | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) (limited to 'content_scripts/marks.coffee') diff --git a/content_scripts/marks.coffee b/content_scripts/marks.coffee index 5bb35f1f..3b5814da 100644 --- a/content_scripts/marks.coffee +++ b/content_scripts/marks.coffee @@ -36,15 +36,15 @@ Marks = # characters. @exit => if event.shiftKey - chrome.runtime.sendMessage - handler: 'createMark' - markName: keyChar - scrollX: window.scrollX - scrollY: window.scrollY - , => @showMessage "Created global mark", keyChar + chrome.runtime.sendMessage + handler: 'createMark' + markName: keyChar + scrollX: window.scrollX + scrollY: window.scrollY + , => @showMessage "Created global mark", keyChar else - localStorage[@getLocationKey keyChar] = @getMarkString() - @showMessage "Created local mark", keyChar + localStorage[@getLocationKey keyChar] = @getMarkString() + @showMessage "Created local mark", keyChar activateGotoMode: (registryEntry) -> @mode = new Mode -- cgit v1.2.3 From 8eda31fb236b9506d577a25908d4e5334d3289d5 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Fri, 5 Jun 2015 07:07:02 +0100 Subject: Global marks; global marks are always recorded for the top frame only. With global marks, we may later create a new tab when the mark is used. When doing so, we should always be using the URL and scroll position of the top frame within the tab. For example, if a global mark is created from within the Hangouts frame of Google's Inbox and the mark is later used, then we should create a new tab for Inbox's URL and scroll position, not for the Hangout's URL and scroll position. --- content_scripts/marks.coffee | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'content_scripts/marks.coffee') diff --git a/content_scripts/marks.coffee b/content_scripts/marks.coffee index 3b5814da..f569e1ea 100644 --- a/content_scripts/marks.coffee +++ b/content_scripts/marks.coffee @@ -36,11 +36,14 @@ Marks = # characters. @exit => if event.shiftKey + # We record the current scroll position, but only if this is the top frame within the tab. + # Otherwise, we'll fetch the scroll position of the top frame from the background page later. + [ scrollX, scrollY ] = [ window.scrollX, window.scrollY ] if DomUtils.isTopFrame() chrome.runtime.sendMessage handler: 'createMark' markName: keyChar - scrollX: window.scrollX - scrollY: window.scrollY + scrollX: scrollX + scrollY: scrollY , => @showMessage "Created global mark", keyChar else localStorage[@getLocationKey keyChar] = @getMarkString() -- cgit v1.2.3 From 83fc0e58f6139ff5a1ae37fc300ea647da079171 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Fri, 5 Jun 2015 07:14:50 +0100 Subject: Global marks; always treat ` and ' as local marks. On some keyboards (who knows?) "`" or "'" could be shift keys. In this case, with the previous implementation, these would be treated as global marks and `` would be unusable. This commit fixes that problem. --- content_scripts/marks.coffee | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) (limited to 'content_scripts/marks.coffee') diff --git a/content_scripts/marks.coffee b/content_scripts/marks.coffee index f569e1ea..067d05a8 100644 --- a/content_scripts/marks.coffee +++ b/content_scripts/marks.coffee @@ -23,6 +23,12 @@ Marks = showMessage: (message, keyChar) -> HUD.showForDuration "#{message} \"#{keyChar}\".", 1000 + # If is depressed, then it's a global mark, otherwise it's a local mark. This is consistent + # vim's [A-Z] for global marks and [a-z] for local marks. However, it also admits other non-Latin + # characters. The exceptions are "`" and "'", which are always considered local marks. + isGlobalMark: (event, keyChar) -> + event.shiftKey and keyChar not in @previousPositionRegisters + activateCreateMode: -> @mode = new Mode name: "create-mark" @@ -31,11 +37,8 @@ Marks = suppressAllKeyboardEvents: true keypress: (event) => keyChar = String.fromCharCode event.charCode - # If is depressed, then it's a global mark, otherwise it's a local mark. This is consistent - # vim's [A-Z] for global marks, [a-z] for local marks. However, it also admits other non-Latin - # characters. @exit => - if event.shiftKey + if @isGlobalMark event, keyChar # We record the current scroll position, but only if this is the top frame within the tab. # Otherwise, we'll fetch the scroll position of the top frame from the background page later. [ scrollX, scrollY ] = [ window.scrollX, window.scrollY ] if DomUtils.isTopFrame() @@ -58,7 +61,7 @@ Marks = keypress: (event) => @exit => keyChar = String.fromCharCode event.charCode - if event.shiftKey + if @isGlobalMark event, keyChar chrome.runtime.sendMessage handler: 'gotoMark' markName: keyChar -- cgit v1.2.3