Generalize md_browser to work on other repos

Generalize md_browser to work on other repos

R=dpranke@chromium.org

Committed: https://crrev.com/d40b9548bb48d09d6a1dab7499ad610b7eb74ba2
Cr-Commit-Position: refs/heads/master@{#392804}

[Dirk Pranke, 2016-05-06 01:23:41]

https://codereview.chromium.org/1952533002/diff/1/tools/md_browser/md_browser.py
File tools/md_browser/md_browser.py (right):

https://codereview.chromium.org/1952533002/diff/1/tools/md_browser/md_browser...
tools/md_browser/md_browser.py:34: print(" Try loading
http://localhost:%s/docs/README.md" % args.port)
Does it still make sense to execute line 34 if args.directory != SRC_DIR ?

https://codereview.chromium.org/1952533002/diff/1/tools/md_browser/md_browser...
tools/md_browser/md_browser.py:93: if not
full_path.startswith(self.server.top_level):
self.server.top_level isn't guaranteed to be an absolute path the way you've
changed things. It needs to be for this to be work and enforce the corresponding
security principle.

https://codereview.chromium.org/1952533002/diff/1/tools/md_browser/md_browser...
tools/md_browser/md_browser.py:146: def _ReadBoilerplate(self, relpath):
maybe just change _Read to take a second, optional, parameter, called top_level
or some such that defaults to self.server.top_level but can be overridden to
SRC_DIR on line 157 ?

[agable, 2016-05-09 21:00:01]

https://codereview.chromium.org/1952533002/diff/1/tools/md_browser/md_browser.py
File tools/md_browser/md_browser.py (right):

https://codereview.chromium.org/1952533002/diff/1/tools/md_browser/md_browser...
tools/md_browser/md_browser.py:34: print(" Try loading
http://localhost:%s/docs/README.md" % args.port)
On 2016/05/06 at 01:23:41, Dirk Pranke wrote:
> Does it still make sense to execute line 34 if args.directory != SRC_DIR ?

Good point, put in something smarter.

https://codereview.chromium.org/1952533002/diff/1/tools/md_browser/md_browser...
tools/md_browser/md_browser.py:93: if not
full_path.startswith(self.server.top_level):
On 2016/05/06 at 01:23:41, Dirk Pranke wrote:
> self.server.top_level isn't guaranteed to be an absolute path the way you've
changed things. It needs to be for this to be work and enforce the corresponding
security principle.

Fixed, it becomes an abspath at Server.__init__ time.

https://codereview.chromium.org/1952533002/diff/1/tools/md_browser/md_browser...
tools/md_browser/md_browser.py:146: def _ReadBoilerplate(self, relpath):
On 2016/05/06 at 01:23:41, Dirk Pranke wrote:
> maybe just change _Read to take a second, optional, parameter, called
top_level or some such that defaults to self.server.top_level but can be
overridden to SRC_DIR on line 157 ?

Done.

[Dirk Pranke, 2016-05-10 20:35:36]

lgtm.

https://codereview.chromium.org/1952533002/diff/40001/tools/md_browser/md_bro...
File tools/md_browser/md_browser.py (right):

https://codereview.chromium.org/1952533002/diff/40001/tools/md_browser/md_bro...
tools/md_browser/md_browser.py:152: relative_to = self.server.top_level
nit: it'd be a bit more idiomatic (and one less line) to write:

  relative_to = relative_to or self.server.top_level

[agable, 2016-05-11 00:26:25]

https://codereview.chromium.org/1952533002/diff/40001/tools/md_browser/md_bro...
File tools/md_browser/md_browser.py (right):

https://codereview.chromium.org/1952533002/diff/40001/tools/md_browser/md_bro...
tools/md_browser/md_browser.py:152: relative_to = self.server.top_level
On 2016/05/10 at 20:35:35, Dirk Pranke wrote:
> nit: it'd be a bit more idiomatic (and one less line) to write:
> 
>   relative_to = relative_to or self.server.top_level

That disallows passing "relative_to=''", which would be silly, but could make
sense if someone is pulling shenanigans in /.

[agable, 2016-05-11 00:26:26]

The CQ bit was checked by agable@chromium.org

[commit-bot: I haz the power, 2016-05-11 00:26:59]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1952533002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1952533002/40001

[Dirk Pranke, 2016-05-11 00:32:55]

https://codereview.chromium.org/1952533002/diff/40001/tools/md_browser/md_bro...
File tools/md_browser/md_browser.py (right):

https://codereview.chromium.org/1952533002/diff/40001/tools/md_browser/md_bro...
tools/md_browser/md_browser.py:152: relative_to = self.server.top_level
On 2016/05/11 00:26:25, agable wrote:
> On 2016/05/10 at 20:35:35, Dirk Pranke wrote:
> > nit: it'd be a bit more idiomatic (and one less line) to write:
> > 
> >   relative_to = relative_to or self.server.top_level
> 
> That disallows passing "relative_to=''", which would be silly, but could make
> sense if someone is pulling shenanigans in /.

Hm. That seems like something I wouldn't want to support, but it's not that
important, so we can leave as-is for now.

[commit-bot: I haz the power, 2016-05-11 01:15:21]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-05-11 01:16:27]

Description was changed from

==========
Generalize md_browser to work on other repos

R=dpranke@chromium.org
==========

to

==========
Generalize md_browser to work on other repos

R=dpranke@chromium.org

Committed: https://crrev.com/d40b9548bb48d09d6a1dab7499ad610b7eb74ba2
Cr-Commit-Position: refs/heads/master@{#392804}
==========

[commit-bot: I haz the power, 2016-05-11 01:16:28]

Patchset 3 (id:??) landed as
https://crrev.com/d40b9548bb48d09d6a1dab7499ad610b7eb74ba2
Cr-Commit-Position: refs/heads/master@{#392804}