don't have seq and cmd as commandgroups

[neogeographica]

Unfortunately it seems like it would be a good idea now to make a change that will churn a lot of the top-level code.

Currently the invocation syntax requires you to do "chaintool seq" or "chaintool cmd" followed by an operation (like "run").

A couple of those operations ("print" and "vals") also exist as top-level commandgroups where they do "apply to all commands".

But now that cmd and seq names can't conflict with each other, the only time you should REALLY need to specify that a single-item operation is for seq or cmd is when you are creating a new item. This means we could e.g. just do "chaintool run" instead of "chaintool seq/cmd run", etc.

Need to think about a few consequences of that including:

• Locking will change a bit, as we'd always grab both inventory locks (to see if a name is a cmd or seq) and then maybe drop one of them.
• What's the best way to handle create-new-thing? Currently "set"/"edit" will automatically do "create if doesn't exist, otherwise update". If I go to this new scheme though, create needs to be told if it is seq or cmd, while update does not. Or maybe it would still be a good sanity check for update, so you don't clobber the wrong thing?
• The top level "print" and "vals" operations could now very naturally accept a list of seqs and/or cmds, so that they don't have to be restricted to applying to either single-item or all-commands. That's especially nice for "vals".

(The place where it would be most painful to do this change is probably in the bash completion script. It would be an opportunity to revisit/clean/comment that code a bit, I guess.)

[neogeographica]

Another gotcha for combining cmd/seq operations would be any cases where they currently have different optional flags.

• Seq set/edit have the --force flag, and cmd does not.

• Cmd del has the --force flag, and seq does not. (But seq might in the future if/when chaintool supports seq-containing-seq.)

• Seq run has --ignore-errors and --skip, and cmd does not.

Might be a sign that it's not as natural to collapse these together as I was thinking?

It's possible to finesse these in various ways for combined cmd/seq ops. I.e.:

• Always allow the --force flag on set, edit, and del. Just in some cases it's not ever needed.

• Always allow --ignore-errors on run. It's not super useful for cmd but I guess it at least can suppress any error exit status.

• Maybe just get rid of --skip? Not 100% convinced it was a good idea. <== yeah do this

Perhaps though I'd be painting myself into the corner if the future turns up other ways in which ops should be different for cmd vs. seq.

[neogeographica]

OK. Instead of "set" and "edit" let's just have "edit", which is used only for update of existing. It can either take the item's content as a single optional positional argument (like "set" currently does, at least for cmd), or if that is omitted it does the interactive editing (like "edit" currently does).

For creating a new item, let's have "new cmd" and "new seq", which otherwise behave the same as "edit".

The top-level ops replacing seq/cmd commandgroups would then look like:

list [-c] [itemtype]
print itemname
new [-f] [-q] itemtype itemname [content]
edit [-f] [-q] itemname [content]
vals [-q] itemname [arg ...]
del [-f] itemname [itemname ...]
run [-i] [-q] itemname [arg ...]

The next question is what to do with the functionality from the current top-level "print" and "vals" ops. For the all-print, we could support having no "itemname" arg to represent that case (and also support multiple specified itemnames). The all-vals op is trickier (as is multiple-specified-itemnames) because we have other kinds of positional args there. Need to think about that more.

The top-level "import", "export", and "x" ops would be unaffected.

[neogeographica]

Strawman proposal:

For the "print", "vals", "del", and even (although less usefully) "run" operations, replace "itemname" with "itemspec".

This can still be a simple item name. However it can also be an fnmatch-style pattern. Or it could be a space-separated list of names and patterns. Obviously in the latter cases it would need to be quoted, because we need to protect special characters and whitespace (this is still just a single argument).

So your itemspec for "vals" could be a single sequence like 'foo', or it could be multiple things like 'foo bar', or it could be everything '*', etc.

This is pretty easy to implement:

import fnmatch
def spec_matches(names, spec):
    results = set()
    for specpart in spec.split():
        results.update(fnmatch.filter(names, specpart))
    return results

A sample run of that:

# "names" would be from the combined cmd and seq name lists
names = ['q3build', 'q3build-simple', 'q3aas', 'q3light']
# 'spec' is the inputspec from the invocation
spec = 'q3b* q3aas'

print(spec_matches(names, spec))

{'q3build', 'q3aas', 'q3build-simple'}

Although hmm, it would be more in the spirit of ls-like behavior to preserve any available ordering (and duplicates). So, more like:

import fnmatch
def spec_matches(names, spec):
    results = []
    for specpart in spec.split():
        results += fnmatch.filter(names, specpart)
    return results

names = ['q3build', 'q3build-simple', 'q3aas', 'q3light']
spec = 'q3b* q3aas'

print(spec_matches(names, spec))

['q3build', 'q3build-simple', 'q3aas']

Misc notes:

• Allowing this for the "run" op just for consistency. Essentially this could be used to define a sequence to run on-the-fly. Although if you use matching patterns you don't have control over the order of the resulting list.

• For things like "print" and "del" that could easily be made to accept multiple itemnames as independent arguments, do we let them accept multiple itemspecs now? Because one itemspec can cover multiple itemnames anyway. I think allowing multiple arguments is still nice in those cases, because in the common case where you're just specifying a few item names (not using patterns) autocomplete can still help you fill those out.

If all that pans out, then the ops would be like:

list [-c] [itemtype]
print itemspec [itemspec ...]
new [-f] [-q] itemtype itemname [content]
edit [-f] [-q] itemname [content]
vals [-q] itemspec [arg ...]
del [-f] itemspec [itemspec ...]
run [-i] [-q] itemspec [arg ...]

[neogeographica]

Possible function to do both spec interpretation and locking (haven't tried to run or even syntax check it yet):

def process_itemspecs(specs, inv_locktype, items_locktype):
    """Find and lock items matched by specs.

    blah blah blah

    :param specs:          list of item specs for matching seqs/cmds
    :type specs:           list[str]
    :param inv_locktype:   how to lock the seq and cmd inventory
    :type inv_locktype:    LockType.WRITE | LockType.READ
    :param items_locktype: how to lock the matched seqs and cmds
    :type items_locktype:  LockType.WRITE | LockType.READ

    :returns: a 3-tuple; first element the list of matches, second element
              a set of all affected commands, third element a dict of the
              command list for each matched sequence
    :rtype:   (list[str], set[str], dict[str, list[str]])

    """
    patterns = " ".join(specs).split()
    matches = []
    cmds_for_seqs = dict()
    matched_seqs = set()
    affected_cmds = set()
    locks.inventory_lock("seq", inv_locktype)
    seqs = sequence_impl_core.all_names()
    for p in patterns:
        matched_seqs.update(fnmatch.filter(seqs, p))
    locks.multi_item_lock("seq", matched_seqs, items_locktype)
    for seq in matched_seqs:
        seq_dict = sequence_impl_core.read_dict(seq)
        seq_cmds = seq_dict["commands"]
        cmds_for_seqs[seq] = seq_cmds
        affected_cmds.update(seq_cmds)
    locks.inventory_lock("cmd", inv_locktype)
    cmds = command_impl_core.all_names()
    all_items = seqs + cmds
    for p in patterns:
        affected_cmds.update(fnmatch.filter(cmds, p))
        filter_results = fnmatch.filter(all_items, p)
        filter_results.sort()
        matches += filter_results
    locks.multi_item_lock("cmd", affected_cmds, items_locktype)
    return (matches, affected_cmds, cmds_for_seqs)

The ops would use that function as follows.

print:

• call process_itemspecs with READ, READ
• release both inventory locks
• for each match
  • if is in cmds_for_seqs, do print_multi with the dict value
  • otherwise do print_one with the match

vals:

• call process_itemspecs with READ, WRITE
• release both inventory locks
• apply vals to affected cmds set
• if pretty-printing:
  • for each match:
    • if is in cmds_for_seqs, do print_multi with the dict value
    • otherwise do print_one with the match

del:

• call process_itemspecs with WRITE, WRITE
• for each match:
  • if is in cmds_for_seqs, delete seq with the match
  • otherwise delete cmd with the match

run:

• call process_itemspecs with READ, READ
• release both inventory locks
• for each match
  • if is in cmds_for_seqs, run commands list in the dict value
  • otherwise run the matched command

[neogeographica]

For seq content, it would be quite nice to be able to supply item spec(s) there as well. Even with an option as to whether or not to expand them now or later (when the seq is used).

Of course an item spec can match a sequence name, which brings up issue #7. Could just forbid that, but possibly could commit to solve issue #7 instead.

[neogeographica]

Going to be a minute before I can pick this back up, so saving some additional notes here:

• Disallow special fnmatch chars in item names.
• In spec, if no fnmatch chars, it is an item name -- don't do match. Error if not exist.
• For print, each item spec is "set of commands to run" (so, printed like seq).
  • Therefore should NOT collapse multiple specs together.
• For seq contents, for now only allow commands names ... does dict format need to be forward-looking though? (for eventually maybe containing seq names, specs, pipes, etc.)