Skip to content

Add ordinal scope types #13

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
6 of 16 tasks
Tracked by #414
pokey opened this issue Jun 23, 2021 · 20 comments · Fixed by #987
Open
6 of 16 tasks
Tracked by #414

Add ordinal scope types #13

pokey opened this issue Jun 23, 2021 · 20 comments · Fixed by #987
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@pokey
Copy link
Member

pokey commented Jun 23, 2021

So could say "take next dollar fine" to select the first $ after the given token. Could also say "<ordinal> <symbol>", eg "second dollar".

In addition, could support "next line fine", "third line fine", etc, as well as "next funk fine", etc

For convenience, potentially "down <number>" could be alias for "<ordinal> line".

Would need to support other direction as well, eg "take last dollar fine", or "take second to last dollar fine", tho the latter is a bit verbose

Use "prior" for opposite of "next"

We should also figure out compact grammar for the following:

  • Support simple relative and absolute ordinal scope modifiers #982
  • Scope and next scope
  • Scope and next two scopes
  • Every scope from scope through end of iteration scope
  • Every scope from scope through start of iteration scope
  • Scope and previous scope
  • Scope and previous two scopes
  • Target past next scope
  • This past next scope
  • Target past previous scope
  • This past previous scope
  • Target past next two scopes (might not need to be compact)
  • How to make these work with "next <symbol>" eg "next comma"
  • How to make these work with "next instance <target>" for selecting next occurrence of a target
    • Does this work for any target or just mark?
    • Where does it start searching? At <target> or cursor or able to search from a different target?

"next <mark>"

It should always be next instance of given mark's text relative to input. So eg if there is a token "hello" with a hat over the h, then "next harp" will select the next instance of hello after the cursor

Should support "every", "first", etc for these as well

Note that for things like :, ,, ), etc, we don't really care about the token that has the hat because it will always just be the one character, so it will behave like we want anyway. For failure modes of this assumption (eg < and <=), we can add something like glyph modifier or something in the future if that becomes a problem

Plurals

Where plurals make sense (eg "three funks"), vendor in the following sections of inflection:

And have csv overrides code generate a list called foo_plural if you pass in a flag asking for plurals

Some proposed spoken forms:

IMG_0044

@AndreasArvidsson
Copy link
Member

I would really like the down 3 aproach as well. Then we could do stuff like:

take up 5
chuck down 5
take up 5 past down 2
take up 5 past blue air
move funk blue air to down 5

@pokey pokey added the enhancement New feature or request label Jun 23, 2021
@pokey
Copy link
Member Author

pokey commented Jun 23, 2021

@AndreasArvidsson Yes and given it's a transformation rather than a mark we can also say "take fine up 5 past down 2"

@AndreasArvidsson
Copy link
Member

@auscompgeek Had a really interesting idea "take every third funk"

@pokey
Copy link
Member Author

pokey commented Sep 22, 2022

@auscompgeek Had a really interesting idea "take every third funk"

What would that do?

@pokey
Copy link
Member Author

pokey commented Sep 22, 2022

Proposed shape: let's use the thing we have today with modifications; see #13 (comment)

interface OrdinalScope {
    type: "ordinalScope"

    scopeType: ScopeType; 

    /* The start of the range.  If negative, go backwards from selection if {@link isRelative} is `true`, or start from end of iteration scope if not. */
    start: number;

    /* The number of scopes to include.  Will always be positive.  If greater than 1, will include scopes after {@link start} */
    length: number;

    /* Whether to be relative to target or relative to iteration scope */
    isRelative: boolean;

    /** Indicates that the range should have anchor and active reversed.  If `undefined`, inherit direction from input to this modifier */
    isReversed?: boolean;
}

@pokey
Copy link
Member Author

pokey commented Sep 22, 2022

Fwiw this is the version of the type today:

export interface OrdinalRangeModifier {
type: "ordinalRange";
scopeType: ScopeType;
anchor: number;
active: number;
excludeAnchor?: boolean;
excludeActive?: boolean;
}

Maybe we just add isRelative to that and be done with it?

Keep in mind that if it is absolute, we need to look at whether input has explicit range to decide whether it's iteration scope vs input range

@AndreasArvidsson
Copy link
Member

One downside of the current format is that there is no way of knowing if this was a previous or a next command when using relative. The problem is that if you already have two scopes selected we want to take the one after the last selected or the one before the first selected. Maybe we can calculate this but it would be easier if we had that information in the api.

@pokey
Copy link
Member Author

pokey commented Sep 22, 2022

I'm not sure I understand. If anchor and active are both 1, then it is "next", and if they are both -1 then it is "previous". If you have two scopes selected, then "next" should be the one after each of them (so two scopes), and "previous" should be the one before each of them (also two scopes). Why would we only want the last one selected? That's inconsistent with all of our other modifiers imo. If you have two selections, then the pipeline will just call this modifier on each of them individually

@AndreasArvidsson
Copy link
Member

AndreasArvidsson commented Sep 22, 2022

I wasn't clear. The problem is calculating the current relative index. If I already have multiple continuously selected which one is the next? I would argue that it is the next one after the last one already selected.

@pokey
Copy link
Member Author

pokey commented Sep 22, 2022

What do you mean by "current relative index"?

Ah when you say "multiple", you mean a continuous range. I thought you meant multiple selections. Yeah it should be as you said: "next" is after the last selected, and "previous" is before the first selected. Not sure I see the problem with that?

@AndreasArvidsson
Copy link
Member

[1, |2, 3|, 4]
"take next item" should of course select 4 but for that to work we must first know that we should use 3 as reference and not 2

@pokey
Copy link
Member Author

pokey commented Sep 22, 2022

Why can't the modifier stage just handle that?

@AndreasArvidsson
Copy link
Member

The problem is that the api is not clear about this. I guess I can calculate it thou

@AndreasArvidsson
Copy link
Member

I think I have most inference regarding isRelative down but there is still some ambiguity regarding relative index 0.
[1, |2, 3|, 4]

isRelative : true,
anchor: 0,
active: 0

Is this referring to 2 or 3?

A bit of a weird edge case, but I'm wondering if we should do relative: "previous" instead to make it clearer.

@pokey
Copy link
Member Author

pokey commented Sep 22, 2022

I'd say it refers to both, but tbh it's just containing scope, and I also don't think there's any spoken form that spits this out, right?

@AndreasArvidsson
Copy link
Member

AndreasArvidsson commented Sep 23, 2022

I guess I could see that relative index 0 should be all that intersects with current target/selection. I do believe we would get this with "take 1 funks". A bit weird to select multiple functions when you specified only one though.
If index zero refers to a single of your previous multi selection we also have some ambiguity with "2 funks" vs whatever we use for it's previous counterpart. Both of them would contain 0.

Also I'm wondering how expressive we want the grammar to be? should you be able to say "take previous past next funk"? "take previous past fifth funk"? Because in that case we're actually mixing relative with absolute. I'm not saying that we need to support that kind of grammar, but I would like the extension side to don't limit it if we want in the future.

I think I would prefer:

anchorRelative?: "previous" | "next";
activeRelative?: "previous" | "next";

That would give us the power to combine absolute and relative as well as removing the need to infer relative direction.

@pokey
Copy link
Member Author

pokey commented Sep 23, 2022

Hmm. I wonder if we want to switch to using proper compound targets. Then we just need fields for relative and index. I wonder if the fact that this modifier bakes in range info is just an accident of the way our Talon grammar is defined

@AndreasArvidsson
Copy link
Member

Maybe? The only reason for keeping the range inside its own modifier is if we want to have a separate implementation for it. If we just wanted to behave the same as any other range then using a compound target would probably be easier. In that case our sub tokens would just be normal containing/every scopes and we would have the same implementation for the ordinals. We should probably discuss this on tomorrow's meeting?

@pokey
Copy link
Member Author

pokey commented Sep 25, 2022

Representation:

  • For any grammatical construct that uses "past", use a proper range target (and list target for "and" once we support that)
  • The primitive target will be as follows:
interface AbsoluteOrdinalScope {
    type: "absoluteOrdinalScope"

    scopeType: ScopeType; 

    /* The start of the range.  Start from end of iteration scope if `start` is negative */
    start: number;

    /* The number of scopes to include.  Will always be positive.  If greater than 1, will include scopes after {@link start} */
    length: number;
}

interface RelativeOrdinalScope {
    type: "relativeOrdinalScope"

    scopeType: ScopeType; 

    /* Indicates how many scopes away to start relative to the input target.  Note that if {@link direction} is `"backward"`, then this scope will be the end of the output range.  */
    offset: number;

    /* The number of scopes to include.  Will always be positive.  If greater than 1, will include scopes in the direction of {@link direction} */
    length: number;

    /* Indicates which direction both `offset` and `length` go relative to input target  */
    direction: "forward" | "backward";
}

@AndreasArvidsson AndreasArvidsson self-assigned this Sep 26, 2022
@pokey pokey changed the title Support "next <symbol>" / "next <selectionType>" / "next <scopeType>" Add ordinal scope types Sep 29, 2022
@AndreasArvidsson AndreasArvidsson linked a pull request Oct 2, 2022 that will close this issue
5 tasks
@AndreasArvidsson
Copy link
Member

@pokey Should we reopen this since several of the tasks are not completed or should we create a follow up issue?

@pokey pokey reopened this Oct 7, 2022
@AndreasArvidsson AndreasArvidsson removed their assignment Oct 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants