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

redrawhook issues #245

Closed
danielshahaf opened this issue Nov 30, 2015 · 13 comments
Closed

redrawhook issues #245

danielshahaf opened this issue Nov 30, 2015 · 13 comments

Comments

@danielshahaf
Copy link
Member

danielshahaf commented Nov 30, 2015

The following issues should be fixed by the zsh redrawhook branch (#154 (comment)):
#90 #150 #154 #183 #203 #377

(confirmed for #90 and #150; the rest are known to be widget-wrapping-related issues, and the branch does away with widget wrapping)

The following issue probably won't be:
#40

edit: Upstream committed a patch that, in in conjunction with this issue, fixes #40, q.v.

The following issues might be fixed by using the zle -f yank / zle -f kill feature already in upstream zsh:
#150 #183

edit: #375 is also reported to be fixed by redrawhook branch, and #500 is suspected to be (not tested).

@danielshahaf
Copy link
Member Author

The zsh redrawhook branch was merged upstream in zsh-users/zsh@c3ea3ff, to appear in zsh-5.2.1 zsh-5.3.

@danielshahaf
Copy link
Member Author

Note potential issue with print -zr: workers/38499

@danielshahaf
Copy link
Member Author

Upstream is considering introducing an add-zsh-hook-like API for hooking to zle-line-pre-redraw: workers/38499

@danielshahaf
Copy link
Member Author

Upstream is considering introducing an add-zsh-hook-like API for hooking to zle-line-pre-redraw: workers/38499

Done: workers/38670

@danielshahaf
Copy link
Member Author

danielshahaf commented Jun 15, 2016

I have drafted a patch that uses workers/38670 here: master...danielshahaf:mikachu-redrawhook/v3 At the moment it requires zsh master + two patches from workers@, but I expect both of these patches will be applied to zsh master soon enough.

See also #261 and #261 (comment) for two implementations that precede add-zle-hook-widget.

edit: see users/21671 and its thread for context.

edit: This branch is now #356.

@danielshahaf
Copy link
Member Author

danielshahaf commented Jul 22, 2016

Status update:

There are two candidate implementations:

  • The one from my last comment.

    Uses add-zle-hook-widget.

  • @m0vie's use zle-line-pre-redraw hook if available #261; a part of it had been factored out into fixes for zle handling #288.

    Predates add-zle-hook-widget, uses generic wrapping instead. These two PRs also have some improvements of independent interest (e.g., use of zle-line-finish)

    Update: [These branches conflict, so I've merged them onto a temporary head and resolved conflicts: https://github.com/zsh-users/zsh-syntax-highlighting/compare/master...danielshahaf:tmp/for-i245-merge-i288-and-i261-v1]

So: need to merge one or the other. (#288 needs to be merged in either case.) Acceptance criteria [compare https://github.com//pull/288#issuecomment-223329830]:

Checklist last updated for da60234 (master) + eb506b5 (#356) using zsh zsh-users/zsh@5b4cbcc

danielshahaf pushed a commit to danielshahaf/zsh-syntax-highlighting that referenced this issue Jul 29, 2016
zsh version 5.2 and lower don't support ISEARCHMATCH_ACTIVE and
we are unable to re-apply zle_highlight on top. Therefore it is
impossible to see the underlined matched area.

Since that information is more important, completely disable
highlighting in isearch in that case.

To do that, we need to make sure we are actually called when
something changes in isearch.

Trumps zsh-users#257.

The FAQ entry presupposes zsh-users#245 will be fixed (in time for the release) too.
m0vie added a commit to m0vie/zsh-syntax-highlighting that referenced this issue Aug 24, 2016
@danielshahaf
Copy link
Member Author

Note #261 was just updated: #261 (comment)

@danielshahaf
Copy link
Member Author

Subtotal: 89bf578 addresses all known issues. Review the history, squash if necessary, merge to master, solicit testing on zsh-workers@ (since zsh features from master are used).

danielshahaf added a commit that referenced this issue Oct 17, 2016
Fixes #356 (the pull request being merged).
Fixes #245 (the meta-issue for the following redrawhook issues).
Fixes #40.
Fixes #90.
Fixes #150.
Fixes #154.
Fixes #183.
Fixes #377.

* upstream/pr/356:
  test harness: Actually test the new code.
  driver: Rewrite without a state variable
  noop: Make a whitespace-only change to reduce noise in the next commit.
  docs: Rewrap.
  docs: Update FAQ answer per changes on this branch.
  redo _zsh_highlight__function_callable_p
  driver: Use a different way of checking whether add-zle-hook-widget is present.
  changelog: Use a more specific link.
  changelog: Note the effect of fixing #245/#90 and an alternative.
  driver: Pass zle-line-finish arguments on to _zsh_highlight.
  driver: Hook zle-line-finish.
  driver: Reimplement using 'add-zle-hook-widget zle-line-pre-redraw'
  wrappers: Reimplement using Mikachu's zle-line-pre-redraw hook (workers/36650).
@danielshahaf danielshahaf reopened this Oct 18, 2016
@danielshahaf danielshahaf modified the milestones: 0.5.0, 0.6.0 Oct 19, 2016
@danielshahaf
Copy link
Member Author

Re-milestoned: 0.5.0 will be released as master minus zsh-5.3 features.

@danielshahaf
Copy link
Member Author

Re-milestoned: We're going to release a quick 0.6.0 so this will be deferred to 0.7.0.

phy1729 pushed a commit to phy1729/zsh-syntax-highlighting that referenced this issue Oct 12, 2018
phy1729 pushed a commit to phy1729/zsh-syntax-highlighting that referenced this issue Oct 21, 2018
@danielshahaf danielshahaf modified the milestones: 0.7.0, 0.8.0 Jun 16, 2019
@danielshahaf
Copy link
Member Author

(As this is the umbrella redrawhook issue:)

I just pushed the following:

To github.com:zsh-users/zsh-syntax-highlighting
b08d508..59cb9a5 feature/redrawhook -> feature/redrawhook
d9a7963..8a1bd7c master -> master

Some stats about the first of these:

% git diff --stat b08d508..59cb9a5  | grep -v …
 HACKING.md                                         |   74 +-
 INSTALL.md                                         |   40 +-
 Makefile                                           |    2 +-
 README.md                                          |    5 +
 changelog.md                                       |  364 +++++--
 docs/highlighters.md                               |   53 +-
 docs/highlighters/brackets.md                      |   12 +-
 docs/highlighters/cursor.md                        |    4 +-
 docs/highlighters/line.md                          |    4 +-
 docs/highlighters/main.md                          |   39 +-
 docs/highlighters/pattern.md                       |    9 +-
 docs/highlighters/regexp.md                        |    5 +-
 docs/highlighters/root.md                          |    4 +-
 highlighters/main/main-highlighter.zsh             | 1140 ++++++++++++++------
 highlighters/pattern/pattern-highlighter.zsh       |    2 +-
 highlighters/regexp/regexp-highlighter.zsh         |    2 +-
 zsh-syntax-highlighting.zsh                        |  149 ++-
 172 files changed, 6325 insertions(+), 621 deletions(-)
% git rev-list --count b08d508..59cb9a5 
310
% git show --pretty=fuller $(git rev-list b08d508..59cb9a5 | tail -1) | grep Date:
AuthorDate: Sat Oct 13 08:14:42 2018 -0500
CommitDate: Sat Oct 13 09:37:09 2018 -0500

In other words, this merged to the feature/redrawhook branch some 21 months' worth of changes to master.

Therefore:

  • If we start to get bug reports about sudden regressions from users of feature/redrawhook, that'll be the likely reason.

  • Such bug reports would also be good to know about before we merge feature/redrawhook into master, where they'd affect many more people.

danielshahaf added a commit that referenced this issue Jul 14, 2020
…ixed by this branch.

This covers issues referenced from #245 or labelled "feature:redrawhook" or "widget-wrapping".  See #749 for details.
danielshahaf added a commit that referenced this issue Aug 9, 2020
* feature/redrawhook:
  docs: Track making the new codepath conditional upon the 'memo=' feature.
  On the feature/redrawhook branch, changelog: Add entries for issues fixed by this branch.
  On the feature/redrawhook branch, change the detection of the 'memo=' feature to avoid a catch-22.
  driver: Make the redrawhook codepath conditional upon the memo= feature.
  On the feature/redrawhook branch, move the changelog entry to the current release's section.
  driver: Fix a bug that prevented subsequent, third-party zle-line-pre-redraw hooks from running.
  driver: Do not pass widget arguments to _zsh_highlight
  driver: Clarify comment.  No functional change.
  driver: Allow for -U in autoloaded function definition
  driver: Use idiomatic module check
  driver: Make the shadowing $WIDGET read only.
  driver: Avoid a fork in the common case.
  test harness: Actually test the new code.
  driver: Rewrite without a state variable
  noop: Make a whitespace-only change to reduce noise in the next commit.
  docs: Rewrap.
  docs: Update FAQ answer per changes on this branch.
  redo _zsh_highlight__function_callable_p
  driver: Use a different way of checking whether add-zle-hook-widget is present.
  changelog: Use a more specific link.
  changelog: Note the effect of fixing #245/#90 and an alternative.
  driver: Pass zle-line-finish arguments on to _zsh_highlight.
  driver: Hook zle-line-finish.
  driver: Reimplement using 'add-zle-hook-widget zle-line-pre-redraw'
  wrappers: Reimplement using Mikachu's zle-line-pre-redraw hook (workers/36650).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant