global-auto-rename-tag mode applies too globally 😀

[aaronjensen]

Hi there, awesome package. I really like the similar VS Code package whenever I find myself using VS Code.

Unfortunately for me, this mode appears to add significant latency to every change command. To make matters worse, there seems to be something it does that angers tide-mode and causes tide-mode to continually run tide-net-filter which in turn triggers the auto-rename-tag-before-change-functions. The end result is that when I make a change to a tag with tide-mode enabled, Emacs more or less becomes unusable.

Here's an example profile:

- tide-net-filter                                                8816  51%
 - auto-rename-tag-before-change-functions                       8597  49%
  - auto-rename-tag-inside-tag                                   8597  49%
   - auto-rename-tag-backward-char-at-point                      8597  49%
    - auto-rename-tag-backward-goto-char                         7558  43%
       auto-rename-tag-current-char-equal-p                      6370  36%
 - tide-decode-response                                           218   1%
  - auto-rename-tag-before-change-functions                       194   1%
   - auto-rename-tag-inside-tag                                   194   1%
    - auto-rename-tag-backward-char-at-point                      193   1%
     - auto-rename-tag-backward-goto-char                         192   1%
        auto-rename-tag-current-char-equal-p                      184   1%
      auto-rename-tag-forward-char-at-point                         1   0%
  - tide-dispatch                                                  18   0%
   + tide-dispatch-response                                        18   0%
  + tide-decode-response                                            3   0%
    tide-decode-response-legth                                      2   0%
  + json-read-object                                                1   0%

With tide mode disabled, it's usable, but still chugs as I make changes with a more confusing profile:

- redisplay_internal (C function)                                 994  77%
 - #<compiled 0x41a11657>                                         991  76%
  - window--adjust-process-windows                                991  76%
   - window--process-window-list                                  991  76%
    - walk-windows                                                991  76%
     - #<compiled 0x44b2ba69>                                     991  76%
      - internal--after-save-selected-window                      991  76%
       - select-window                                            991  76%
        - apply                                                   991  76%
         - #<compiled 0x454444c9>                                 785  60%
          - forge-sql                                             785  60%
           - apply                                                785  60%
            - emacsql                                             785  60%
             - apply                                              785  60%
              - #<compiled 0x40c40573>                            785  60%
               - apply                                            785  60%
                - #<compiled 0x460d9eed>                          785  60%
                 - apply                                          785  60%
                  - #<compiled 0x4d90fc81>                        785  60%
                   - apply                                        785  60%
                    - #<compiled 0x4580358d>                      785  60%
                     - emacsql-send-message                       782  60%
                      - apply                                     782  60%
                       - #<compiled 0x41640333>                   782  60%
                        - apply                                   782  60%
                         - #<compiled 0x45fb4871>                 780  60%
                          - emacsql-log                           780  60%
                           - apply                                780  60%
                            - #<compiled 0x45fb4865>                780  60%
                             - princ                              780  60%
                              - auto-rename-tag-before-change-functions                780  60%
                               - auto-rename-tag-inside-tag                780  60%
                                - auto-rename-tag-backward-char-at-point                779  60%
                                 - auto-rename-tag-backward-goto-char                670  51%
                                    auto-rename-tag-current-char-equal-p                523  40%

So, I think there are two issues--the before-change hook is firing in places I wouldn't necessarily expect, and when it does fire, it is slow. Would a looking-back based implementation be faster?

Thanks again for making this and releasing it.

[aaronjensen]

Okay, unraveled the mystery a bit. The problem is that global-auto-rename-tag-mode is too global for me. It applies to hidden buffers used for communicating with processes (like forge and tide are doing) so those are the ones slowing things down. If I disable that and only enable auto-rename-tag-mode on the buffer I'm working on, it's much more usable.

[jcs090218]

Sorry for the late reply!

I wasn't very sure why I design this package with global use case. I think the best use is to just have it enable in certain mode! For instance, web-mode, rjsx-mode, etc.

Hope this helps! :)

[aaronjensen]

Makes sense, thanks!

I just tried it out again and ran into a few other issues. Feel free to open separate issues for them if you like.

• flycheck via lsp-mode doesn't appear to detect the changes to the closing tag, so it shows an error even when there isn't one (this is possibly a bug in flycheck or lsp-mode...)
• Changing the first div in this code made changes to the wrong place:

code

export const NavBarLayout = ({ children, pageTitle }: Props) => (
  <>
    <Head>
      <title>{pageTitle}</title>
    </Head>
    <div className="h-screen flex flex-col overflow-y-hidden">
      <nav className="border-b border-b-4">
        <div className="px-4 sm:px-6 lg:px-8">
          <div className="flex items-center justify-between h-16">
            <div className="flex items-center">
              <div className="flex-shrink-0 text-center flex items-center">
                <img
                  className="mx-auto h-6 w-auto"
                  src="/other-logo.svg"
                  alt="Other Logo"
                />
                <span className="mx-4 text-4xl">+</span>
                <img
                  className="mx-auto h-16 w-auto transform"
                  style={{ '--transform-rotate': '14deg' } as any}
                  src="/logo.png"
                />
              </div>
            </div>
          </div>
        </div>
      </nav>
      <div className="bg-white shadow-sm">
        <div className="py-4 px-4 sm:px-6 lg:px-8">
          <h1 className="text-lg leading-6 font-semibold text-gray-900">
            {pageTitle}
          </h1>
        </div>
      </div>
      <div className="flex flex-1 py-6 overflow-y-hidden bg-cool-gray-100">
        {children}
      </div>
    </div>
  </>
)

[jcs090218]

• flycheck via lsp-mode doesn't appear to detect the changes to the closing tag, so it shows an error even when there isn't one (this is possibly a bug in flycheck or lsp-mode...)

Yeah, I am not quite sure the error here. Even there are error, I don't think this package should change anything to match either lsp or flycheck. But I have opened another issue just to make sure if anything pop out from anyone!

The issue is here #9

• Changing the first div in this code made changes to the wrong place:

code

Resolved, see #8

[jcs090218]

Removed global from this commit b271596. global version of this package doesn't exists anymore so it don't cause confusions! Close this issue! :)