Metadata support tracking & discussion

[cmyr]

Metadata part 1: initial per-line indentation adjustment

overview

My initial goal is going to be to support calculating per-line indentation changes (i.e. whether or not we should increase or decrease the indentation level after a given line). I want to do this properly, as discussed in #179: full scope selectors, matching against a line's trailing scope. Approximate TODOs:

• use full scope selectors for matching
• add metadata dumping/loading
• everything behind a feature flag
• unit tests for a good range of cases
• a simple API suitable for use from a text editor

Subsequent work:

• comment metadata, comment toggling
• real full-file reindent, with a CLI file reindenter?

maybe one day (not currently planned)

• Symbol lists
• other shell variables?
• snippets

I'm very receptive to input/feedback if people think various of these things are more or less important

Some Qs:

1. I'm going to need to add the 'Default' sublime package in order for this to work correctly. Are there any possible issues with this?

2. Support for user settings: I'm thinking specifically of indent_to_bracket. A text editor using the library would probably prefer if it could hand over this setting to us, and let us handle the implementation; this would be easier than, say, having a separate check for a trailing bracket and clobbering our indentation. Are there any other settings like this?

3. I think we'll need to support indentParens and indentSquareBrackets. Is there an argument against doing this? I guess one could argue that it should be a user setting as well. We could perhaps support it being both...

4. I've been thinking about testing a bit, and I'm getting hung up on one particular problem. The following two bits of python code are both valid indentations:

# one
if feeling == "happiness":
    do_a_dance()
get_on_with_life()

# two
if feeling == "happiness":
    do_a_dance()
    get_on_with_life()

Is this handled in any way by the regex-based system, or is there some other mechanism? I took a quick look and none of the regexes seemed to be looking for whitespace; in any case to detect an indentation decrease we would need to be matching at least two lines, I think?

5. Do I need to manually adjust the regexes to include the implicit ^ anchor?

That's all that I can remember, I'm sure more things will come up.

cc @trishume @keith-hall

[keith-hall]

regarding 4, Python is an indentation sensitive language, so the reindent functionality should never be used for it. When working with auto indentation, the user should backspace a level of indentation as and when required. The TextMate indentation system doesn't cater for decreasing indentation after a particular line i.e. pass, one can only get it to unindent pass itself, which is no use.

re 5: We have seen that regex rewriting is a nightmare. Better to use a match method on the regex pattern if it exists, or search and check the index is 0.

in terms of settings, Sublime allows users to override the behaviors, meta data, settings etc. in a granular way - i.e. only for a specific syntax or file etc. We should probably think how we want this interaction to be achieved with syntect as a library when used without user input (i.e. syncat) and with (from an editor like Xi). Having pack dumps makes things a bit more complicated, because it may be that we need to read in user prefs and apply those on top of the premade packs from every endpoint.

[keith-hall]

We could make our own Default package with our own sane defaults, rather than copy Sublime's, as theirs is not open source and it wouldn't be fun trying to keep it in sync, plus not everything there is relevant for us anyway. But then we may also need to copy Sublime's precedence behavior. https://github.com/guillermooo/sublime-undocs/blob/8a3b31288b1e329263c65701b5c133dc8755bc27/source/extensibility/packages.rst#merging-and-order-of-precedence

Sublime's indentation implementation isn't necessarily correct (see the controversial preserveIndent behavior and sublimehq/Packages#1156). So, about indentParens and indentSquareBrackets, has anyone had chance to see how they behave in other editor that use them? (TextMate etc.) . In Sublime, they only affect when indentation is increased, and having it enabled can therefore prevent the rest of the document from ever returning to 0 indentation, depending on the decreaseIndentPattern. Indeed, these 2 settings mainly seem to each add an alternation to the main increaseIndentPattern setting - perhaps better behavior would be to also affect decreaseIndentPattern in a similar way?

[cmyr]

Okay, so I've picked this back up again, and hope to PR something in the next couple of days.

My current plan is to add a pretty narrow set of features to syntect that mostly just expose metadata fields to consumers, handle loading and dumping metadata, etc. Consumers can then do what they want with it; that is, I'm not currently focused on actually implementing auto-indent logic based on metadata in syntect. (I might add a basic example binary that reindents a file, but I want to be generally non-opinionated about API etc.)

I will need to add some Default package, for this, but it can be minimal.

My current inclination is to only include the metadata fields related to indentation and comments, although if anyone has strong feelings I'm happy to add other stuff?

[cmyr]

@keith-hall so I'm playing around with shellVariables, and have a little problem: bincode can't encode/decode serde_json::Value / serde_json::Map, so I can't trivially just have a shellVariables map object.

I can see two immediate options: either ignore shellVariables, and just extract the comment metadata I want, or I could do something like,

struct ShellVariables {
    tm_comment_start: Option<String>,
    tm_comment_end: Option<String>,
    tm_comment_disable_indent: Option<bool>,
    tm_comment_start_2: Option<String>,
    tm_comment_end_2: Option<String>,
    tm_comment_disable_indent_2: Option<bool>,
    tm_comment_start_3: Option<String>,
    tm_comment_end_3: Option<String>,
    tm_comment_disable_indent_3: Option<bool>,
}

Which maybe gets us most of the way there?

[keith-hall]

silly question, but can't we just map it into two separate string arrays, which hopefully can be serialized?

shellVariablesKey[0] = "TM_COMMENT_START"
shellVariablesValue[0] = "/*"
shellVariablesKey[1] = "TM_COMMENT_END"
shellVariablesValue[1] = "*/"

obviously its not as nice to work with if it gets deserialized back into that form, but that should be easy to map back into a more useful datastructure, no?

[cmyr]

I think that could work, and we could have a custom serializer/deserializer? Will certainly complicate the code a little bit, but I'll give it a shot.

[cmyr]

Actually your suggestion points to a much simpler solution, which is just to keep shellVariables as a BTreeMap<String, String> which solves the problem. 🤦‍♂️

[keith-hall]

Thanks Colin, I don't mean to make your life harder :)
my reasoning for wanting this is that metadata can be used for arbitrary things and if we end up having to cater for it later then it would likely be a breaking change ;)

[cmyr]

Always happy to have someone keeping me honest. 😉