Skip to content
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

Feat/node insertion #17

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from
Draft

Feat/node insertion #17

wants to merge 7 commits into from

Conversation

DieracDelta
Copy link
Owner

@DieracDelta DieracDelta commented May 2, 2021

This is a draft PR for AST node insertion. Objectives:

  • create AST nodes for attribute sets
  • convert between Input type and attribute set
  • clean code
    • so many places need &str instead of String
    • better file naming to match rest of project
    • clippy
  • tie into main control loop so we can create inputs
  • more tests

Resolves #18 and #14 .

src/parser/input_utils.rs Outdated Show resolved Hide resolved
GreenNode::new(kind, children)
}

pub fn gen_attr_set(attr_pairs: Vec<(String, String)>) -> GreenNode{
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pub fn gen_attr_set(attr_pairs: Vec<(String, String)>) -> GreenNode{
pub fn gen_attr_set(attr_pairs: &[(String, String)]) -> GreenNode{

Copy link
Owner Author

@DieracDelta DieracDelta May 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm trying to do this, and running into issues here. It looks like there's not an easy way to convert from a vector to an array..

src/parser/input_utils.rs Outdated Show resolved Hide resolved
@DieracDelta
Copy link
Owner Author

DieracDelta commented May 10, 2021

Input generation works, and I've added in a nixpkgs_fmt pass to make sure things look nice. I've added a couple of tests as well. Code quality is poor; lots of unused imports, dead code, etc. A bunch of the processing done in main.rs does not belong. Inputs_utils needs to be split out into something more modular that matches with the rest of the style. Not ready to merge, but getting there.

I also think that a bunch of the functions in utils.rs and input_utils.rs morally speaking belong in rnix. I'm just filling in the gaps done by TODOs. I was hoping for more discussion on the issue I opened there...

@DieracDelta
Copy link
Owner Author

Also, inputs need to be (1) checked, and (2) there should be some sort of type generation. Ideally, we should support other types of flake inputs that covers the entire spec. Path/git/, enabling submodules, revisions, etc. Essentially, everything specified here. I think I'd prefer to do these in separate PRs though. The scaffolding here can define the basis for further work.

src/main.rs Outdated Show resolved Hide resolved
src/main.rs Outdated Show resolved Hide resolved
src/main.rs Outdated Show resolved Hide resolved
src/main.rs Show resolved Hide resolved
Comment on lines 58 to 70
let kind = NixLanguage::kind_to_raw(NODE_KEY_VALUE);
let assign_kind = NixLanguage::kind_to_raw(TOKEN_ASSIGN);
let whitespace_kind = NixLanguage::kind_to_raw(TOKEN_WHITESPACE);
let semicolon_kind = NixLanguage::kind_to_raw(TOKEN_SEMICOLON);
let children = vec![
NodeOrToken::Node(key),
NodeOrToken::Token(GreenToken::new(whitespace_kind, " ")),
NodeOrToken::Token(GreenToken::new(assign_kind, "=")),
NodeOrToken::Token(GreenToken::new(whitespace_kind, " ")),
NodeOrToken::Node(value),
NodeOrToken::Token(GreenToken::new(semicolon_kind, ";")),
NodeOrToken::Token(GreenToken::new(whitespace_kind, "\n")),
];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let kind = NixLanguage::kind_to_raw(NODE_KEY_VALUE);
let assign_kind = NixLanguage::kind_to_raw(TOKEN_ASSIGN);
let whitespace_kind = NixLanguage::kind_to_raw(TOKEN_WHITESPACE);
let semicolon_kind = NixLanguage::kind_to_raw(TOKEN_SEMICOLON);
let children = vec![
NodeOrToken::Node(key),
NodeOrToken::Token(GreenToken::new(whitespace_kind, " ")),
NodeOrToken::Token(GreenToken::new(assign_kind, "=")),
NodeOrToken::Token(GreenToken::new(whitespace_kind, " ")),
NodeOrToken::Node(value),
NodeOrToken::Token(GreenToken::new(semicolon_kind, ";")),
NodeOrToken::Token(GreenToken::new(whitespace_kind, "\n")),
];
use NixLanguage::kind_to_raw;
use NodeOrToken::{Node, Token};
let kind = kind_to_raw(NODE_KEY_VALUE);
let assign_kind = kind_to_raw(TOKEN_ASSIGN);
let whitespace_kind = kind_to_raw(TOKEN_WHITESPACE);
let semicolon_kind = kind_to_raw(TOKEN_SEMICOLON);
let children = vec![
Node(key),
Token(GreenToken::new(whitespace_kind, " ")),
Token(GreenToken::new(assign_kind, "=")),
Token(GreenToken::new(whitespace_kind, " ")),
Node(value),
Token(GreenToken::new(semicolon_kind, ";")),
Token(GreenToken::new(whitespace_kind, "\n")),
];

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't import NixLanguage::kind_to_raw. Not sure why....

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh... (that might be the case because the method is defined in a trait impl)
Maybe create a small wrapper for it then:

#[inline(always)]
fn kind_to_raw(x: rnix::SyntaxKind) -> rowan::SyntaxKind {
    NixLanguage::kind_to_raw(x)
}

src/parser/input_utils.rs Outdated Show resolved Hide resolved
Comment on lines 4 to 6
use rowan::{api::SyntaxNode, GreenNode, GreenNodeBuilder, GreenToken, Language, NodeOrToken};
use std::string::ToString;
use NodeOrToken::{Node, Token};
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
use rowan::{api::SyntaxNode, GreenNode, GreenNodeBuilder, GreenToken, Language, NodeOrToken};
use std::string::ToString;
use NodeOrToken::{Node, Token};
use rowan::{api::SyntaxNode, GreenNode, GreenNodeBuilder, GreenToken, Language, NodeOrToken::{self, Node, Token}};
use std::string::ToString;

@@ -99,26 +94,17 @@ pub fn merge_attr_sets(a1: GreenNode, a2: GreenNode) -> GreenNode {
pub fn new_attr_set(attr_pairs: Vec<(GreenNode, GreenNode)>) -> GreenNode {
let pairs: Vec<NodeOrToken<_, _>> = attr_pairs
.iter()
.map(move |(k, v)| NodeOrToken::Node(new_key_value(k.clone(), v.clone())))
.map(move |(k, v)| Node(new_key_value(k.clone(), v.clone())))
.collect::<Vec<_>>();
let open_curly_kind = NixLanguage::kind_to_raw(TOKEN_CURLY_B_OPEN);
let close_curly_kind = NixLanguage::kind_to_raw(TOKEN_CURLY_B_CLOSE);
let attr_set_kind = NixLanguage::kind_to_raw(NODE_ATTR_SET);
let whitespace_kind = NixLanguage::kind_to_raw(TOKEN_WHITESPACE);
let mut token_vec = Vec::new();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: replace the complete following stuff with this:

    let mut token_vec = Vec::new();
    let mut tokens = vec![
        Token(GreenToken::new(open_curly_kind, "{")),
        Token(GreenToken::new(whitespace_kind, "\n")),
    ];
    tokens.extend(attr_pairs
        .into_iter()
        .map(move |(k, v)| Node(new_key_value(k, v))));
    tokens.push(Token(GreenToken::new(close_curly_kind, "}")));

Comment on lines 135 to 142
let kind: rowan::SyntaxKind = NixLanguage::kind_to_raw(NODE_STRING);
node.start_node(kind);
let start_string_kind: rowan::SyntaxKind = NixLanguage::kind_to_raw(TOKEN_STRING_START);
node.token(start_string_kind, "\"");
let string_content: rowan::SyntaxKind = NixLanguage::kind_to_raw(TOKEN_STRING_CONTENT);
node.token(string_content, &item.val.to_string());
let end_string_kind: rowan::SyntaxKind = NixLanguage::kind_to_raw(TOKEN_STRING_END);
node.token(end_string_kind, "\"");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let kind: rowan::SyntaxKind = NixLanguage::kind_to_raw(NODE_STRING);
node.start_node(kind);
let start_string_kind: rowan::SyntaxKind = NixLanguage::kind_to_raw(TOKEN_STRING_START);
node.token(start_string_kind, "\"");
let string_content: rowan::SyntaxKind = NixLanguage::kind_to_raw(TOKEN_STRING_CONTENT);
node.token(string_content, &item.val.to_string());
let end_string_kind: rowan::SyntaxKind = NixLanguage::kind_to_raw(TOKEN_STRING_END);
node.token(end_string_kind, "\"");
node.start_node(NixLanguage::kind_to_raw(NODE_STRING));
node.token(NixLanguage::kind_to_raw(TOKEN_STRING_START), "\"");
node.token(NixLanguage::kind_to_raw(TOKEN_STRING_CONTENT), &item.val.to_string());
node.token(NixLanguage::kind_to_raw(TOKEN_STRING_END), "\"");

Comment on lines 116 to 118
pub(crate) struct Key {
val: SmlStr,
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a simple newtype struct would be a better fit, unless you intend to add more fields.

Suggested change
pub(crate) struct Key {
val: SmlStr,
}
pub(crate) struct Key(SmlStr);

@DieracDelta
Copy link
Owner Author

DieracDelta commented Jun 13, 2021

@zseri sorry I haven't pulled in your changes yet. I think that in order for this to work in a scalable manner (we'd like to pull in some sort of evaluation mechanism so we can meaningfully make edits and check their correctness), the IR needs to be a bit more flushed out. Going to focus on that for a bit then cleanup in the input code. In essence we're making our own IR here. I think it should at a higher level than the output of the parser, and we should have some sort of mechanism to switch between IR and GreenNodes. We'll parse the structure into this higher level IR, make changes based on user request, then push down to GreenNodes again.

Copy link
Collaborator

@fogti fogti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest splitting the EDSL-like + conversion between that and rnix, rowan into a separate crate (maybe rnix-ddhir), so we can lay out a cleaner interface for it rather sooner than later.

String,
Function,
List(Box<NixType>),
AttrSet(Vec<Box<NixType>>),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how is this AttrSet type decl supposed to work?

Str(SmlStr),
}

trait NixValue {}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not model all possible values here (including rather opaque stuff like function calls)? Maybe it would be a good idea to put this into a separate crate.

let kind: rowan::SyntaxKind = NixLanguage::kind_to_raw(NODE_KEY);
#[derive(Debug, Clone, Eq, PartialEq, Default)]
pub(crate) struct NixAttrSet<T: Into<GreenNode> + Clone> {
kvs: Vec<NixKeyValue<T>>,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why don't we use something like the indexmap crate here?

Copy link
Collaborator

@fogti fogti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

necessary because of the removal of the SmlStr newtype

@@ -0,0 +1,269 @@
use crate::parser::utils::{string_to_node, NixNode};
use crate::SmlStr;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
use crate::SmlStr;
use smol_str::SmolStr;


#[derive(Debug, Clone, Eq, PartialEq, Default)]
pub(crate) struct NixKey {
pub val: SmlStr,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pub val: SmlStr,
pub val: SmolStr,


#[derive(Debug, Clone, Eq, PartialEq, Default)]
pub(crate) struct NixStringLiteral {
pub val: SmlStr,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pub val: SmlStr,
pub val: SmolStr,

Comment on lines +234 to +235
//Key(SmlStr),
//StringLiteral(SmlStr),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
//Key(SmlStr),
//StringLiteral(SmlStr),
//Key(SmolStr),
//StringLiteral(SmolStr),

//let mut inputs = Vec::new();
//if let Some(s) = item.url {
//inputs.push((
//SyntaxStructure::Key(SmlStr::new_inline("url")).into(),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
//SyntaxStructure::Key(SmlStr::new_inline("url")).into(),
//SyntaxStructure::Key(SmolStr::new_inline("url")).into(),

@@ -124,6 +133,8 @@ pub(crate) enum UserPrompt {
#[display("{0}")]
SelectLang(Lang),
#[display("{0}")]
Bool(bool),
#[display("{0}")]
Other(SmlStr),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

??? maybe rebase necessary ???

@@ -0,0 +1,21 @@
use crate::SmlStr;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
use crate::SmlStr;
use smol_str::SmolStr;

enum NixPrimitive {
Bool(bool),
Int(i64),
Str(SmlStr),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Str(SmlStr),
Str(SmolStr),

//let mut inputs_gn = Vec::<Box<dyn>>::new()
//if let Some(url) = input.url {
//inputs_gn.push((
//NixKey{val: SmlStr::new_inline("url")}.into(),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
//NixKey{val: SmlStr::new_inline("url")}.into(),
//NixKey{val: SmolStr::new_inline("url")}.into(),

//let url_gn: GreenNode = url.into();
//}
//if let Some(is_flake) = input.is_flake {
//inputs_gn.push(NixKey{val: SmlStr::new_inline("flake")}.into(), );
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
//inputs_gn.push(NixKey{val: SmlStr::new_inline("flake")}.into(), );
//inputs_gn.push(NixKey{val: SmolStr::new_inline("flake")}.into(), );

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

nixpkgs-fmt
2 participants