Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Quick View: handle subsequent images in url() #3599

Merged
merged 1 commit into from
Apr 24, 2013
Merged

Conversation

redmunds
Copy link
Contributor

This is for #3581. Code was only looking for first url() match.

Head's up: @gruehle, @RaymondLim.

@ghost ghost assigned RaymondLim Apr 24, 2013
tokenString = urlMatch[1];
break;
}
urlMatch = urlRegEx.exec(line);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: you can replace while loop with do ... while and remove this line.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestion, but I don't think it improves the code:

    do {
        urlMatch = urlRegEx.exec(line);
        if (urlMatch && pos.ch >= urlMatch.index && pos.ch <= urlMatch.index + urlMatch[0].length) {
            tokenString = urlMatch[1];
            break;
        }
    } while (urlMatch);

There's 1 less line that calls urlRegEx.exec(line), but there's an extra check for urlMatch, so it checks urlMatch twice on every loop.

I'm going to leave it like it is.

@RaymondLim
Copy link
Contributor

Done reviewing. Just one minor nit.

RaymondLim added a commit that referenced this pull request Apr 24, 2013
Quick View: handle subsequent images in url()
@RaymondLim RaymondLim merged commit 8fb5c9a into master Apr 24, 2013
@RaymondLim RaymondLim deleted the randy/issue-3581 branch April 24, 2013 23:08
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