Skip to content
This repository has been archived by the owner on Oct 5, 2018. It is now read-only.

Implement linescan.rescan() #8

Merged
merged 10 commits into from
Mar 21, 2014
Merged

Implement linescan.rescan() #8

merged 10 commits into from
Mar 21, 2014

Conversation

le717
Copy link
Owner

@le717 le717 commented Mar 18, 2014

It took some work, but I have finally implemented linescan.rescan() from #2.

@le717
Copy link
Owner Author

le717 commented Mar 19, 2014

Hey, @tribex, if you are not too busy, would you take a look at this? It looks and acts correctly to me (and I have tested many use cases), but it never hurts to have a second opinion. If you can't, that's fine too. Just unsubscribe from the notifications. 😉

@JoshTheDerf
Copy link

Sure! starts typing

I'm not entirely sure what the correct behavior should be, but it seems to be working so far... My Python skills have gotten a bit rusty... (Currently working with HTML5 on Andriod) :L

What do you mean by this?

Just unsubscribed from the notifications. 😉

testfile.txt is almost creepy...

EDIT: More complete review,

As far as I can tell, it works fine. However, it is a bit confusing to tell what is actually going on behind-the-scenes. There can only be one linescan per script correct? Is there a way you could convert it to an instance-able class? Or is that intentional?

I honestly don't see why this is easier to use than the normal way, but I may just be missing something.

Triangle717 added 2 commits March 19, 2014 10:12
@le717
Copy link
Owner Author

le717 commented Mar 19, 2014

@tribex Try this:

import linescan

one = linescan.scan("examples/testfile.txt", 1)
two = linescan.scan("examples/testfile.txt", 1, 3)
end = linescan.scan("examples/testfile.txt", 1, "end")

print(one)
print(two)
print(end)

one = linescan.scan("examples/testfile.txt", 12)
print(one)
print(two)

This shows there can be more than one instance. Before the current revision (there were two other implementations before the current one), performing multiple scans overwrote the last one.
I have changed testfile.txt's content. 😉

rescan() allows you to update previously scanned files. Example, if line 1 of testfile.txtchanged between the two readings (after print but before reassignment to line 12), recalling it would report the old data. Running rescan("examples/testfile.txt"), then, would update the dictionary.

I gave some details as to why I wrote this in http://wp.me/p1V5ge-1nO (or can you not access my blog?).

@le717
Copy link
Owner Author

le717 commented Mar 19, 2014

@tribex BTW, here is the documentation on rescan().

rescan() allows you to update previously scanned files. Example, if line 1 of testfile.txt changed between the two readings (after print but before reassignment to line 12), recalling it would report the old data. Running rescan("examples/testfile.txt"), then, would update the dictionary.

I just realized I might want to add optional lineno and endline parameters for fine-tuned rescanning. Right now, it rescans all previous scans for filename.

What do you mean by this?
"Just unsubscribed from the notifications. 😉"

Typo. If you did not want to look at it, click the "Unsubscribe" near the top right of the issue to stop receiving notifications for this pull request.

@JoshTheDerf
Copy link

Ah, I see now, although it seems like an odd way to instance things. Why the ten line/scan limit?

And yes, I can't access your blog. 😦

@le717
Copy link
Owner Author

le717 commented Mar 20, 2014

Copy of blog post with rough formatting: https://gist.github.com/le717/b43b023f9a6234016682

linecache also uses a dictionary for instancing, and recall this is inspired by linecache (I wish I could think this is an improved version of linecache save the imported modules searching part :P).

It was an arbitrary number I "deducted". It can be changing using debug(), but I could change the default value.

@JoshTheDerf
Copy link

Ah, thanks, that makes a bit more sense. Looking good. :D

@le717
Copy link
Owner Author

le717 commented Mar 21, 2014

Thanks for looking it over. :)

Last questions: 1. Should I raise the limit from 10 to say, 20? 2. Should I consider adding optional lineno and endline parameters for finer control?

I'll go ahead and merge this and make any other changes after you reply. Thanks again. :)

le717 pushed a commit that referenced this pull request Mar 21, 2014
Implement linescan.rescan()
@le717 le717 merged commit 39b5dca into master Mar 21, 2014
@le717 le717 mentioned this pull request Mar 21, 2014
@le717 le717 deleted the 2-rescan branch March 21, 2014 01:31
@JoshTheDerf
Copy link

No problem! Glad to be of service!

Maybe you should add an optional parameter for the limit or something. I personally think that more parameters are better, but it might go against the idea of being simple and easy-to-use, your call. 😀

@le717
Copy link
Owner Author

le717 commented Mar 31, 2014

Maybe you should add an optional parameter for the limit or something.

I did that in the initial version. See debug(). 😉

Again, thanks for the review. 😄

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

Successfully merging this pull request may close these issues.

2 participants