Skip to content
Snippets Groups Projects

topic: allow specifying current topic using period

Open Sushil Khanchi requested to merge topic/default/cmd-topic-feature into branch/default
3 unresolved threads

This means hg topic . --rev <revspec> will amend the revs with current active topic. This would be useful in general.

Also, it can be provided as an "explicit" alternative to hg amend to change the topic of parent with current active topic.

Closes #44

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
804 804 topic = topic.strip()
805 805 if not topic:
806 806 raise error.Abort(_(b"topic name cannot consist entirely of whitespaces"))
807 # Have some restrictions on the topic name just like bookmark name
808 scmutil.checknewlabel(repo, topic, b'topic')
809
810 rmatch = re.match(br'[-_.\w]+', topic)
811 if not rmatch or rmatch.group(0) != topic:
812 helptxt = _(b"topic names can only consist of alphanumeric, '-'"
813 b" '_' and '.' characters")
814 raise error.Abort(_(b"invalid topic name: '%s'") % topic, hint=helptxt)
807
808 if topic == b'.' and touchedrevs:
  • What if we don't abort here? hg up . works fine, what's the downside of having hg topics . working as a no-op?

    Asking to try and avoid checking touchedrevs inside if topic block, because we are kinda already checking topic inside if touchedrevs block, and having things live in multiple places could lead to messy code.

  • I don't have any strong opinion, but a little inclined to the current one for msg clarity. If we do no-op here and rely on later check, abort msg would be: abort: changing topic requires a topic name or --clear which obviously not wrong, but also doesn't directly say that there is no active topic (for your hg topic . usage)

  • @av6 what do you think? ^

  • Heptapod put one of my previous comments in limbo, so my comment above is confusing. What I meant is to do

    if topic == b'.':
        topic = repo.currenttopic

    and not check touchedrevs here. Don't indent the block that checks topic name, and let it all keep working with or without touchedrevs. Basically make hg topic . not complain about topic name, but instead be a no-op.

    Edited by Anton Shestakov
  • Please register or sign in to reply
  • 805 805 if not topic:
    806 806 raise error.Abort(_(b"topic name cannot consist entirely of whitespaces"))
    807 # Have some restrictions on the topic name just like bookmark name
    808 scmutil.checknewlabel(repo, topic, b'topic')
    809
    810 rmatch = re.match(br'[-_.\w]+', topic)
    811 if not rmatch or rmatch.group(0) != topic:
    812 helptxt = _(b"topic names can only consist of alphanumeric, '-'"
    813 b" '_' and '.' characters")
    814 raise error.Abort(_(b"invalid topic name: '%s'") % topic, hint=helptxt)
    807
    808 if topic == b'.' and touchedrevs:
    809 # amend revisions with current active topic
    810 topic = repo.currenttopic
    811 if not topic:
    812 raise error.Abort(_(b"no active topic to amend"))
    • This sounds like we're amending topic, like changing the active topic or something like that. I don't know how to phrase this better, but maybe simple "no active topic" is enough in this case.

    • Please register or sign in to reply
  • Anton Shestakov assigned to @khanchi97 and unassigned @av6

    assigned to @khanchi97 and unassigned @av6

  • Sushil Khanchi added 11 commits

    added 11 commits

    Compare with previous version

  • (rebased)

  • Sushil Khanchi added 1 commit

    added 1 commit

    • 3fa590f4d720 - topic: allow specifying current topic using period

    Compare with previous version

  • Sushil Khanchi added 1 commit

    added 1 commit

    • 1195fdf27ee1 - topic: allow specifying current topic using period

    Compare with previous version

  • (removed extra whitespaces in a line and pushed updated tests)

  • Sushil Khanchi assigned to @av6 and unassigned @khanchi97

    assigned to @av6 and unassigned @khanchi97

  • 430 430 o 25:7f26084dfaf1 {bar}
    431 431 | Added f ()
    432 432 ~
    433
    434 when no active topic
    435 $ hg topic --clear
    436 $ hg topic . -r "desc('Added h')"
    • Doesn't this look somewhat confusing because . here is not like topic(.), but more like topic(wdir())? And I'm not even sure since when plain . is not only "current revision", but also "current whatever depending on context".

      Also reminds me of . usage in hg bookmarks (that's never good). There, at least, it was a possible argument to --rename, --delete or --list. Without a flag like that, hg topic . looks like it could be equivalent to hg topic --rev . until you see that there's already --rev there.

      So I'd say I'm -0 on this feature. I'd rather keep . meaning "current revision" only.

    • Please register or sign in to reply
    Please register or sign in to reply
    Loading