dykstrom / basic-mode

Emacs major mode for editing BASIC code
GNU General Public License v3.0
7 stars 10 forks source link

Teach basic-find-jumps to handle "on v go(to|sub) x,y,z" #6

Closed pederkl closed 6 years ago

pederkl commented 6 years ago

Hi, it's me again :) I hate loose ends. So here is a slight improvement to the function that scans the buffer for jumps. Now it should to the right thing with the on gotos.

dykstrom commented 6 years ago

Hi, yes your changes work fine! Now, also the on-goto statements are renumbered. :-) However, I did notice that the problem with the last line(s) not being renumbered is not completely fixed, and I don't think this was affected by your last changes. In some cases the last line or lines are not renumbered. Funny thing is that it depends on how I renumber. If I renumber with parameters 100 and 10 the last two lines are not renumbered. If I renumber with 10 and 10 they are. I have a small test file that you can have if you want. Another thing, what do you think about gotos and gosubs in comments and strings? I don't know, but I think comment and string are usually not affected when you refactor code.

pederkl commented 6 years ago

Sorry it's taken a while, it's been a busy week.

On Wed, Nov 22 2017 at 13:41, Johan Dykström wrote:

However, I did notice that the problem with the last line(s) not being renumbered is not completely fixed, and I don't think this was affected by your last changes. In some cases the last line or lines are not renumbered. Funny thing is that it depends on how I renumber. If I renumber with parameters 100 and 10 the last two lines are not renumbered. If I renumber with 10 and 10 they are.

This sounds like the code is getting confused about where it is in the buffer. Which will happen if the size of the buffer changes while we are renumbering (more/fewer digits in the line numbers) while the code hangs on to a buffer position calculated before the size changed.

That is exactly the issue I fixed in PR#7, but I believe that fix will only affect regions. If you are seeing this on whole-buffer renumbering, there must be some other case that I missed.

I have a small test file that you can have if you want.

Yes, please! I'll try to reproduce on my own as well, but I haven't seen it so far, so a working reproducer would be good.

Another thing, what do you think about gotos and gosubs in comments and strings? I don't know, but I think comment and string are usually not affected when you refactor code.

True, but it can be useful, I think, and is easier (less code) than detecting comments and skipping them. I can't think of a case that would have me put "goto 10" in a comment and not want it renumbered if line 10 is suddenly line 20. If we skip renumbering in comments, I believe more comments would be out of sync and misleading post-renumbering than if we renumber. So it should at least be configurable.

Maybe a case would be if you are referring to some specific line in another program? My guess is that would be rare. I would wait until the user complaints start rolling in before spending time fixing that. Maybe open an issue, so we don't forget that it's an area for possible improvement? :)

pederkl commented 6 years ago

I think I cracked the skipped lines problem. See latest commit on PR#7.

dykstrom commented 6 years ago

Yes, your latest commit fixed the renumbering problem. I tested with a file that did not work with the latest version in MELPA, and it now works.

And you're right, there are cases when you do want your comments to be renumbered as well, so we leave it as it is. When the user complaints start rolling in (from the 80 or so users that have downloaded the package so far) we can think of making it configurable. :-)

Thanks for all your good work! If you happen to think of something more that you feel is missing, don't hesitate to let me know or create a new PR. :-)

I have worked on a simple template function that is almost ready. The user could type for example "ie" followed by C-c C-t to insert an IF-THEN-ELSE statement, correctly indented and with line numbers.

Wishing you a nice weekend!

pederkl commented 6 years ago

Thanks, you too!

About the template function - I usually use YASnippet for small templates. Is a separate system for basic-mode warranted?

Don't worry, if I think of any other improvements, I'll send you a patch. But now I need to stop playing around in elisp and actually program some basic! :)

dykstrom commented 6 years ago

I better check out YASnippet then, so I don't spend time on something none will use... Good luck with the Basic programming!