coldnew / pangu-spacing

emacs minor-mode to add space between Chinese/Japanese/Korean and English characters.
144 stars 16 forks source link

Performance #28

Closed yangsheng6810 closed 5 years ago

yangsheng6810 commented 5 years ago

This is patch for a major performance improvement. Performance for pangu-spacing has always been an issue, especially for large files. With this patch, I am able to open a 30MB file with pangu-spacing-mode on.

The main idea is to use the (beg end) provided by jit-lock-mode, and only update those regions. One issue with this patch is that if you delete everything after the last pangu spacing, this spacing will persist. This is probably because there is no overlay after the last pangu spacing, and jit-lock-mode does not report the change. I tried adding a dummy overlay at the end, but it does not seem to work.

Personally, I feel OK with this limitation (anyway, it is recommend to have a line break at the end of a text file), and a workaround may be beyond my reach. It would be great if you can help with this limitation.

dsdshcym commented 5 years ago

Hi, there!

I think this change may have broken the behavior of this package:

For example, after upgrading, this piece of text looks weird:

跟alex解释 task 弹性的问题
image

Notice there is an extra space between a and l and an extra space between 解释 and task

rolling back to previous version fixed this issue

yangsheng6810 commented 5 years ago

@dsdshcym Could you explain the exact steps to reproduce this? I can't reproduce this by inserting the given text in a buffer with pangu-spacing on. Also I noticed the overline and underline of the text. Could it be a source of conflicts?

dsdshcym commented 5 years ago

@yangsheng6810 sorry, I can't explain how to reproduce this.

I just upgraded pangu-spacing yesterday, and typed these texts, then it was like this.

I'm on emacs 26.2, not sure if this info helps.

The overline and underline is from solarized theme for org-mode.

I also tried in *scratch* buffer with text-mode, but it has the same issue.

coldnew commented 5 years ago

@dsdshcym can you test your text with latest pangu-spacing and emacs -q ?

just launch:

emacs -q pangu-spacing.el

then M-x eval-buffer, and load your text.

This will help us debug this issue

dsdshcym commented 5 years ago

@coldnew still the same...

image

Here is the steps I took:

  1. opened emacs with emacs -q pangu-spacing.el
  2. M-x eval-buffer
  3. M-x switch-to-buffer *scratch*
  4. M-x pangu-spacing-mode
  5. paste 跟alex解释 task 弹性的问题
yangsheng6810 commented 5 years ago

@dsdshcym OK, I can reproduce this. Let me take a look.

yangsheng6810 commented 5 years ago

@dsdshcym Could you test if #30 works?

coldnew commented 5 years ago

@dsdshcym Can you help us test current master branch, after PR #30 I think the problems gone.

dsdshcym commented 5 years ago

Yes, it works! Thank you all for this quick fix @yangsheng6810 @coldnew