Skip to content
This repository has been archived by the owner on Dec 28, 2021. It is now read-only.

Comments on nodes #1744

Merged
merged 20 commits into from
Aug 2, 2021
Merged

Comments on nodes #1744

merged 20 commits into from
Aug 2, 2021

Conversation

mwu-tow
Copy link
Contributor

@mwu-tow mwu-tow commented Jul 30, 2021

Pull Request Description

Important Notes

Checklist

Please include the following checklist in your PR:

  • The CHANGELOG.md was updated with the changes introduced in this PR.
  • The documentation has been updated if necessary.
  • All code conforms to the Rust style guide.
  • All code has automatic tests where possible.
  • All code has been profiled where possible.
  • All code has been manually tested in the IDE.
  • All code has been manually tested in the "debug/interface" scene.
  • All code has been manually tested by the PR owner against our test scenarios.
  • All code has been manually tested by at least one reviewer against our test scenarios.

@mwu-tow mwu-tow changed the title Comments on nods Comments on nodes Jul 30, 2021
src/rust/ide/lib/ast/impl/src/macros.rs Outdated Show resolved Hide resolved
src/rust/ide/lib/ast/impl/src/macros.rs Outdated Show resolved Hide resolved
@@ -0,0 +1,33 @@
use crate::prelude::*;

// FIXME move to other parser-related tests
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remember to remove ALL Fixmes

src/rust/ide/src/double_representation/comment.rs Outdated Show resolved Hide resolved
src/rust/ide/src/double_representation.rs Outdated Show resolved Hide resolved
src/rust/ide/src/double_representation/node.rs Outdated Show resolved Hide resolved
@@ -287,7 +466,7 @@ mod tests {
let larg = Ast::var("foo");
let rarg = Ast::var("bar").with_id(id);
let line_ast = Ast::infix(larg,ASSIGNMENT,rarg);
let mut node = NodeInfo::from_line_ast(&line_ast).unwrap();
let mut node = NodeInfo::from_main_line_ast(&line_ast).unwrap();

assert_eq!(node.repr(), "foo = bar");
assert_eq!(node.id(),id);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Test for iterating over nodes in line list.

src/rust/ide/src/double_representation/graph.rs Outdated Show resolved Hide resolved
src/rust/ide/src/controller/graph.rs Outdated Show resolved Hide resolved
@mwu-tow mwu-tow marked this pull request as ready for review August 2, 2021 00:51
@mwu-tow mwu-tow requested a review from wdanilo as a code owner August 2, 2021 00:51
@farmaazon farmaazon self-requested a review August 2, 2021 08:05
@@ -630,7 +640,6 @@ pub struct BlockLine <T> {
}



Copy link
Collaborator

Choose a reason for hiding this comment

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

restore line.

Comment on lines 1229 to 1242
impl <T> BlockLine<Option<T>> {
pub fn transpose(self) -> Option<BlockLine<T>> {
let off = self.off;
self.elem.map(|elem| BlockLine {elem,off})
}

pub fn transpose_ref(&self) -> Option<BlockLine<&T>> {
self.as_ref().map(Option::as_ref).transpose()
}

pub fn map_opt<U>(self, f:impl FnOnce(T) -> U) -> BlockLine<Option<U>> {
self.map(|elem| elem.map(f))
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you have warnings about missing documentation?

Comment on lines +1269 to +1274
/// Iterate over non-empty lines, while keeping their absolute indices.
pub fn enumerate_non_empty_lines(&self) -> impl Iterator<Item=(usize,BlockLine<&T>)> + '_ {
self.iter_all_lines().enumerate().filter_map(|(index,line):(usize,BlockLine<Option<&T>>)| {
let non_empty_line = line.transpose()?;
Some((index, non_empty_line))
})
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why we cannot use non-associated enumerate_non_empty_lines here?

@@ -664,7 +657,10 @@ impl Handle {
false
}
};
let range = NodeIndex::range(node_to_be_after.index,node_to_be_before.index);

// FIXME block lines are not necessarily in a block
Copy link
Collaborator

Choose a reason for hiding this comment

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

Fixme left.

@iamrecursion iamrecursion merged commit fdeed13 into develop Aug 2, 2021
@iamrecursion iamrecursion deleted the wip/mwu/comments-on-nodes branch August 2, 2021 10:27
MichaelMauderer added a commit that referenced this pull request Aug 5, 2021
farmaazon added a commit that referenced this pull request Aug 5, 2021
mwu-tow added a commit to enso-org/enso that referenced this pull request Oct 30, 2021
mwu-tow pushed a commit to enso-org/enso that referenced this pull request Oct 30, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants