Page MenuHomeSoftware Heritage
Differential D1375

Phabricator/plugin: Add support for commented links
ClosedPublic
Actions

Authored by ardumont on Apr 9 2019, 2:12 PM.

Details

Reviewers
vlorentz
olasd
Group Reviewers
Reviewers
Commits
R154:d6ea2105c3b4: Explicit regexp fields
R154:a29ca311e8ec: phab/plugin: Change to match reality, there is at most 1 fragment
R154:b811c0f4e960: Phabricator/plugin: Recognize object id even in lower case
R154:39b83e748073: plugin: Factorize full uri computation in get_object_by_phid
R154:10a188841c8a: Refactor: Break out of the loop prior to parse some more the id
R154:85630dbab7d4: Refactor: Use regexp to fetch object type, id and fragment
R154:abbf2f5c9ed3: Phabricator/plugin: Add support for commented links
Summary

No idea if that works as i have no idea on how to test this.

Hoping for some clues from you people!

Diff Detail

Repository
R154 Limnoria plugins
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 5430
Build 7356: arc lint + arc unit

Event Timeline

ardumont created this revision.Apr 9 2019, 2:12 PM
Harbormaster completed remote builds in B5223: Diff 4418.Apr 9 2019, 2:12 PM
ardumont updated this revision to Diff 4419.Apr 9 2019, 2:14 PM

do'h - Fix 0-0 to 0-9

Harbormaster completed remote builds in B5224: Diff 4419.Apr 9 2019, 2:14 PM
ardumont planned changes to this revision.Apr 9 2019, 2:18 PM
ardumont updated this revision to Diff 4420.Apr 9 2019, 2:33 PM

Transmit object_fragment to object formatter as well

So that they can build the right url

Harbormaster completed remote builds in B5225: Diff 4420.Apr 9 2019, 2:33 PM
ardumont planned changes to this revision.Apr 9 2019, 4:07 PM
ardumont updated this revision to Diff 4568.Apr 13 2019, 4:58 PM
  • Phabricator/plugin: Add support for commented links
  • Refactor: Break out of the loop prior to parse some more the id
  • Refactor: Use regexp to fetch object type, id and fragment

@olasd i finally grasped what you meant, thanks.
I iterated over multiple implementations.
In the end, i chose to use regexp consistently.

Harbormaster completed remote builds in B5430: Diff 4568.Apr 13 2019, 4:58 PM
ardumont updated this revision to Diff 4569.Apr 13 2019, 5:02 PM
  • Explicit regexp fields
Harbormaster completed remote builds in B5431: Diff 4569.Apr 13 2019, 5:02 PM
ardumont updated this revision to Diff 4573.Apr 15 2019, 10:48 AM
  • Phabricator/plugin: Recognize object id even in lower case
Harbormaster completed remote builds in B5435: Diff 4573.Apr 15 2019, 10:48 AM
vlorentz accepted this revision.Apr 15 2019, 10:51 AM

Could you factorize this part:

url = full_XXX['uri']
if object_fragment is not None:
    url = '%s%s' % (url, object_fragment)

by moving it in get_object_by_phid?

Phabricator/plugin.py
224

Replace the last * by a ?, there's at most one fragment.

This revision is now accepted and ready to land.Apr 15 2019, 10:51 AM
ardumont added inline comments.Apr 15 2019, 10:51 AM
Phabricator/plugin.py
230

@vlorentz asked for this, so the bot can hint even lowered-case object

ardumont marked an inline comment as done.Apr 15 2019, 10:53 AM
ardumont added inline comments.
Phabricator/plugin.py
224

Right!

ardumont updated this revision to Diff 4574.Apr 15 2019, 10:59 AM
ardumont marked an inline comment as done.
  • phab/plugin: Change to match reality, there is at most 1 fragment
  • plugin: Factorize full uri computation in get_object_by_phid
Harbormaster completed remote builds in B5436: Diff 4574.Apr 15 2019, 11:00 AM
Closed by commit R154:abbf2f5c9ed3: Phabricator/plugin: Add support for commented links (authored by ardumont). · Explain WhyApr 15 2019, 2:22 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSize
Phabricator/
37 lines

Diff 4568

View Options

Phabricator/plugin.py

Show First 20 Lines • Show All 101 Lines • ▼ Show 20 Lines def conduit(self, recipient):
host = self.registryValue( host = self.registryValue(
'phabricatorURI', channel=recipient 'phabricatorURI', channel=recipient
) )
token = self.registryValue( token = self.registryValue(
'phabricatorConduitToken', channel=recipient 'phabricatorConduitToken', channel=recipient
) )
return self.conduit_for_host_token(host, token) return self.conduit_for_host_token(host, token)
def get_object_by_phid(self, recipient, phid, skip_cache=False): def get_object_by_phid(self, recipient, phid,
skip_cache=False, object_fragment=None):
objs = self.get_objects_by_phid(recipient, [phid], skip_cache) objs = self.get_objects_by_phid(recipient, [phid], skip_cache)
return objs.get(phid) obj = objs.get(phid)
if obj and object_fragment is not None:
url = obj['uri']
obj['uri'] = '%s%s' % (url, object_fragment)
return obj
def get_objects_by_phid(self, recipient, phids, skip_cache=False): def get_objects_by_phid(self, recipient, phids, skip_cache=False):
objects = {} objects = {}
if not skip_cache: if not skip_cache:
for phid in phids: for phid in phids:
obj, timeout = self._phid_object_cache[recipient, phid] obj, timeout = self._phid_object_cache[recipient, phid]
if timeout > time.time(): if timeout > time.time():
objects[phid] = obj objects[phid] = obj
▲ Show 20 Lines • Show All 94 Lines • ▼ Show 20 Lines def diff_status_theme(self, status):
return status return status
def phabricator_object_from_regexp(self, irc, msg, match): def phabricator_object_from_regexp(self, irc, msg, match):
r""" r"""
(?: (?:
^ # start of line ^ # start of line
|(?<![:])\b # word boundary, not preceded by ":" |(?<![:])\b # word boundary, not preceded by ":"
) )
[A-Z]\d+ ([a-zA-Z]) # object type
(\d+) # object id
vlorentzUnsubmitted
Done
Inline Actions

Replace the last * by a ?, there's at most one fragment.

vlorentz: Replace the last `*` by a `?`, there's at most one fragment.
ardumontAuthorUnsubmitted
Done
Inline Actions

Right!

ardumont: Right!
(#\d+[-A-Za-z0-9]*)? # optional comment link (as fragment)
(?: (?:
$ # end of line $ # end of line
|\b # word boundary |\b # word boundary
) )
""" """
for recipient in msg.args[0].split(','): for recipient in msg.args[0].split(','):
object_tag = match.group(0) object_type = match.group(1).upper()
ardumontAuthorUnsubmitted
Done
Inline Actions

@vlorentz asked for this, so the bot can hint even lowered-case object

ardumont: @vlorentz asked for this, so the bot can hint even lowered-case object
object_type = object_tag[0]
object_id = int(object_tag[1:])
lookup = { lookup = {
'B': self.conduit(recipient).harbormaster.build.search, 'B': self.conduit(recipient).harbormaster.build.search,
'D': self.conduit(recipient).differential.revision.search, 'D': self.conduit(recipient).differential.revision.search,
'P': self.conduit(recipient).paste.search, 'P': self.conduit(recipient).paste.search,
'T': self.conduit(recipient).maniphest.search, 'T': self.conduit(recipient).maniphest.search,
} }
if object_type not in lookup: if object_type not in lookup:
return return
object_id = int(match.group(2))
object_fragment = match.group(3) # Or None as this is optional
lookup_args = { lookup_args = {
'constraints': { 'constraints': {
'ids': [object_id], 'ids': [object_id],
}, },
} }
query = lookup[object_type](**lookup_args) query = lookup[object_type](**lookup_args)
r = query.response r = query.response
if not r['data']: if not r['data']:
return return
object = r['data'][0] object = r['data'][0]
formatter = { formatter = {
'B': self.build_formatter, 'B': self.build_formatter,
'D': self.diff_formatter, 'D': self.diff_formatter,
'P': self.paste_formatter, 'P': self.paste_formatter,
'T': self.task_formatter, 'T': self.task_formatter,
} }
self.wrapped_message( self.wrapped_message(
irc.reply, irc.reply,
formatter[object_type](recipient, object), formatter[object_type](recipient, object, object_fragment),
notice=True, notice=True,
prefixNick=False, prefixNick=False,
to=recipient, to=recipient,
) )
def build_formatter(self, recipient, build): def build_formatter(self, recipient, build, object_fragment=None):
full_build = self.get_object_by_phid(recipient, build['phid'], True) full_build = self.get_object_by_phid(recipient, build['phid'], True)
buildable = self.get_buildable(recipient, buildable = self.get_buildable(recipient,
build['fields']['buildablePHID']) build['fields']['buildablePHID'])
details = [] details = []
status = build['fields']['buildStatus']['name'] status = build['fields']['buildStatus']['name']
details.append('status: %s' % self.build_status_theme(status)) details.append('status: %s' % self.build_status_theme(status))
return "{id} for {buildable} ({details}): {title} <{url}>".format( return "{id} for {buildable} ({details}): {title} <{url}>".format(
id=ircutils.bold('B%s' % build['id']), id=ircutils.bold('B%s' % build['id']),
buildable=buildable['buildable']['fullName'], buildable=buildable['buildable']['fullName'],
title=build['fields']['name'], title=build['fields']['name'],
details=', '.join(details), details=', '.join(details),
url=full_build['uri'], url=full_build['uri'],
) )
def diff_formatter(self, recipient, diff): def diff_formatter(self, recipient, diff, object_fragment=None):
full_diff = self.get_object_by_phid(recipient, diff['phid'], True) full_diff = self.get_object_by_phid(recipient, diff['phid'], True,
object_fragment=object_fragment)
repo = self.get_repo(recipient, diff['fields']['repositoryPHID']) repo = self.get_repo(recipient, diff['fields']['repositoryPHID'])
details = [] details = []
details.append( details.append(
'author: %s' % self.get_user_by_phid( 'author: %s' % self.get_user_by_phid(
recipient, diff['fields']['authorPHID'] recipient, diff['fields']['authorPHID']
) )
) )
status = diff['fields']['status']['name'] status = diff['fields']['status']['name']
details.append(self.diff_status_theme(status)) details.append(self.diff_status_theme(status))
return "{id} ({details}) on {repo}: {title} <{url}>".format( return "{id} ({details}) on {repo}: {title} <{url}>".format(
id=ircutils.bold('D%s' % diff['id']), id=ircutils.bold('D%s' % diff['id']),
repo=ircutils.bold(self.get_repo_name(repo)), repo=ircutils.bold(self.get_repo_name(repo)),
title=diff['fields']['title'], title=diff['fields']['title'],
details=', '.join(details), details=', '.join(details),
url=full_diff['uri'], url=full_diff['uri'],
) )
def paste_formatter(self, recipient, paste): def paste_formatter(self, recipient, paste, object_fragment=None):
full_paste = self.get_object_by_phid(recipient, paste['phid'], True) full_paste = self.get_object_by_phid(recipient, paste['phid'], True,
object_fragment=object_fragment)
details = [] details = []
details.append( details.append(
'author: %s' % self.get_user_by_phid( 'author: %s' % self.get_user_by_phid(
recipient, paste['fields']['authorPHID'] recipient, paste['fields']['authorPHID']
) )
) )
return "{id} ({details}): {title} <{url}>".format( return "{id} ({details}): {title} <{url}>".format(
id=ircutils.bold('P%s' % paste['id']), id=ircutils.bold('P%s' % paste['id']),
title=paste['fields']['title'], title=paste['fields']['title'],
details=', '.join(details), details=', '.join(details),
url=full_paste['uri'], url=full_paste['uri'],
) )
def task_formatter(self, recipient, task): def task_formatter(self, recipient, task, object_fragment=None):
full_task = self.get_object_by_phid(recipient, task['phid'], True) full_task = self.get_object_by_phid(recipient, task['phid'], True,
object_fragment=object_fragment)
details = [] details = []
details.append( details.append(
'submitter: %s' % self.get_user_by_phid( 'submitter: %s' % self.get_user_by_phid(
recipient, task['fields']['authorPHID'] recipient, task['fields']['authorPHID']
) )
) )
if task['fields']['ownerPHID']: if task['fields']['ownerPHID']:
details.append( details.append(
▲ Show 20 Lines • Show All 167 Lines • Show Last 20 Lines
About · Developer information · Legal