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

Upgrade the event #567

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Upgrade the event #567

wants to merge 2 commits into from

Conversation

TagFreettle
Copy link

No description provided.

For some reason it keeps on trying to merge multiple lines into one, that is not what I pasted into it, GitHub is messing up the formatting of my script. Somebody needs to fix this.
This version has more features such as:
*Go to specified line in the script.
 *Test the script without saving it.
 *Jump to a line containing a specified string, phrase, or word.
 *Replace instances of a certain string.
 *Add a bookmark and go to the bookmark.
 *A line marker in the left side of the screen.
@Wojbie
Copy link
Contributor

Wojbie commented Aug 14, 2018

Any screenshots? Description?

@SquidDev
Copy link
Contributor

SquidDev commented Aug 15, 2018

I'll echo Wojbie here - the commit messages do contain some details, but it's quite hard to tell what something will look like. See #390 and #396 for some examples of other PRs which touch edit's functionality.

A couple of other points on the code:

  • There's been a lot of aesthetic changes to the code which make it rather hard to tell what's actually changed. For instance, lots of variables have been renamed and whitespace has been rather messed around. While the existing whitespace is definitely inconsistent, fixing that is best left for another PR!
  • I'm really not sure what repairString is for. I can see what it does, but I don't understand the need.
  • Similarly, eliminatePoint - this seems to do little more than tostring the input. I suspect you're trying to strip the decimal point.
  • There's some behaviour which I'm sure makes sense, but seems a little odd, such as the status scrolling code. It may be worth adding a few comments within the code to explain how it works.

Your fork is well over a year old now (you're 200+ commits behind master!) and so there's some conflicts. It's probably worth rebasing onto the latest version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants