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(doc): append after function description the link for the tests #555

Merged
merged 4 commits into from
Nov 4, 2024

Conversation

Mte90
Copy link
Member

@Mte90 Mte90 commented Oct 29, 2024

Ref: #500

The markdown generated:

## `includes`

pub fun includes(array, value) 


Checks if a value is in the array.



You can check the original tests for code examples: [includes_empty_text_array.ab](https://github.com/amber-lang/amber/blob/master/src/tests/stdlib/includes_empty_text_array.ab), [includes_empty_num_array.ab](https://github.com/amber-lang/amber/blob/master/src/tests/stdlib/includes_empty_num_array.ab), [includes_exact_match.ab](https://github.com/amber-lang/amber/blob/master/src/tests/stdlib/includes_exact_match.ab), [includes_partial_match_with_expanded_element.ab](https://github.com/amber-lang/amber/blob/master/src/tests/stdlib/includes_partial_match_with_expanded_element.ab), [includes_text_array.ab](https://github.com/amber-lang/amber/blob/master/src/tests/stdlib/includes_text_array.ab), [includes_prefix_match.ab](https://github.com/amber-lang/amber/blob/master/src/tests/stdlib/includes_prefix_match.ab).

The code basically check if we are using the amber dev version, in this way can get the tests and generate the list.

My rust code will be horrible for sure.

@Mte90 Mte90 requested review from hdwalters and Ph0enixKM October 29, 2024 11:47
@Mte90 Mte90 linked an issue Oct 29, 2024 that may be closed by this pull request
@Mte90
Copy link
Member Author

Mte90 commented Oct 29, 2024

The latest change generate this markdown:

## `array_first_index`

```import { array_first_index } from "std/array.ab"```

```ab
pub fun array_first_index(array, value): Num 

Returns index of the first value found in the specified array.

If the value is not found, the function returns -1.

let mut test_reference = String::from("You can check the original tests for code examples: ");
let mut found_test = false;
if exe_path.contains("target/debug") {
let file_std = meta.context.path.as_ref().and_then(|p| Path::new(&p).file_name()?.to_str().map(|s| s.to_string())).unwrap_or_default();
Copy link
Member Author

Choose a reason for hiding this comment

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

I got that with chatgpt I wasn't able to find a way from Some("./src/std/array.ab") to get only array.ab

Copy link
Member

Choose a reason for hiding this comment

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

okay im not exactly sure what exactly is wrong but there is something wrong with this line.. i will take a closer look tomorrow

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah probably my code can written better :-P

result.push("```ab".to_string());
result.push(self.doc_signature.to_owned().unwrap());
result.push("```\n".to_string());
if let Some(comment) = &self.comment {
result.push(comment.document(meta));
}

if found_test {
test_reference.truncate(test_reference.len() - 2);
Copy link
Member Author

Choose a reason for hiding this comment

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

this to remove the last , so I can add a dot to end the sentence.

Copy link
Member

@b1ek b1ek Oct 29, 2024

Choose a reason for hiding this comment

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

what about .trim_end_matches(" ,") so it will remove only when it is exactly the , at the end

also if you're joining an array of strings theres a Vec<String>.join() for that

Cargo.toml Show resolved Hide resolved
.gitignore Show resolved Hide resolved
Copy link
Member

@b1ek b1ek left a comment

Choose a reason for hiding this comment

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

i think its ok

@b1ek
Copy link
Member

b1ek commented Oct 30, 2024

@hdwalters you can commit those changes yourself in github web editor

@Mte90 Mte90 requested a review from hdwalters October 30, 2024 09:07
@hdwalters
Copy link
Contributor

@hdwalters you can commit those changes yourself in github web editor

Apparently I was able to push to the branch on @Mte90's fork from the command line.

result.push(format!("## `{}`", self.name));
result.push(format!("## `{}`\n", self.name));

let references = self.create_reference(meta, &mut result);
Copy link
Contributor

Choose a reason for hiding this comment

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

I moved the new code to a helper function, which returns an optional string vector (encapsulating both the success of the function, and the additional lines to be added at end of section).

result.join("\n")
}
}

impl FunctionDeclaration {
#[cfg(debug_assertions)]
Copy link
Contributor

@hdwalters hdwalters Oct 30, 2024

Choose a reason for hiding this comment

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

This annotation ensures the new code is only compiled for debug builds, so we don't need to test whether the path contains target/debug. The function annotated with the complementary #[cfg(not(debug_assertions))] below hard codes the return value to None for release builds.

Copy link
Member Author

Choose a reason for hiding this comment

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

how we can compile a debug build? cargo build locally is enough? just to check the CI if needs something to not compile that

Copy link
Contributor

Choose a reason for hiding this comment

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

Use cargo build for a debug build, cargo build --release for a release build.

let mut references = Vec::new();
let exe_path = env::current_exe()
.expect("Executable path not found");
let root_path = exe_path.parent()
Copy link
Contributor

@hdwalters hdwalters Oct 30, 2024

Choose a reason for hiding this comment

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

All path operations are now done via Path and PathBuf objects, for reliability and cross-platform support; no hard-coded / path separators any more.

result.push(String::from("```\n"));
if test_path.exists() && test_path.is_dir() {
if let Ok(entries) = fs::read_dir(test_path) {
let pattern = glob::Pattern::new(&format!("{}*.ab", self.name)).unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

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

Glob pattern is only created once, rather than for each entry in the directory.

.and_then(OsStr::to_str)
.map(String::from)
.unwrap_or_default();
result.push(String::from("```ab"));
Copy link
Contributor

@hdwalters hdwalters Oct 30, 2024

Choose a reason for hiding this comment

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

Triple backticks are put on their own lines, and include an ab annotation, so the output is consistent with other code blocks.

let path = entry.path();
if let Some(file_name) = path.file_name().and_then(OsStr::to_str) {
if pattern.matches(file_name) {
references.push(format!("* [{}](https://github.com/amber-lang/amber/blob/master/src/tests/stdlib/{})", file_name, file_name));
Copy link
Contributor

Choose a reason for hiding this comment

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

Each test file is put on its own line, in a markdown bullet list.

@hdwalters hdwalters requested a review from b1ek October 30, 2024 21:50
@Ph0enixKM
Copy link
Member

Ph0enixKM commented Nov 4, 2024

So basically Amber compiled in:

  • debug mode 👉 this is a standard library
  • release mode 👉 this project is not a standard library

This is a bit stinky but it's good enough for now. We should design a package.json file in which we could hold metadata values. I was thinking of designing a project.ab file which could be defined as follows:

let name = "my-project"
let version = "1.10.5"

pub on_run(args) {
    // Do something
    trust $ amber run main.ab $
}

@Ph0enixKM
Copy link
Member

Other thing is that we could actually embed the tests into the doc-strings. The usecase for this would be so that developers could easily preview the examples directly from their IDEs:

Screenshot 2024-11-04 at 15 44 54

It would be great if this integrated with some testing logic written directly in Amber in the future.

@Mte90 Mte90 merged commit 9fe0f18 into amber-lang:master Nov 4, 2024
1 check passed
@Mte90 Mte90 deleted the doc-tests branch November 4, 2024 15:04
lens0021 added a commit to lens0021/amber-docs that referenced this pull request Dec 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants