Montser - #3
Conversation
There was a problem hiding this comment.
Pull request overview
This PR primarily improves project documentation by expanding the README and adding a new tests.md checklist of minishell edge cases and expected behaviors; it also includes a header change and adds an empty file_test file.
Changes:
- Replaced the minimal README with a fuller project description, feature list, build/run instructions, and references.
- Added
tests.mdcontaining a structured list of edge-case/manual test scenarios (syntax, expansion, pipes, redirections, signals, etc.). - Introduced a new
t_pidtypedef ininc/minishell.hand added an emptyfile_testfile at repo root.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
README.md |
Expanded documentation: overview, features, usage, resources, and authors. |
tests.md |
Added a manual test matrix for minishell edge cases and expected outputs/status codes. |
inc/minishell.h |
Added a new t_pid type (currently an empty struct). |
file_test |
Added an empty file named file_test at repository root. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| | [✅] `cat \| cat \| cat \| echo` | ` ` (empty line) | 0 | ✅ | | ||
| | [✅] `cat Makefile \| base64 \| xxd -p \| xxd -r -p \| base64 --decode` | Makefile contents | 0 | ✅ | | ||
| | [✅] `cat Makefile \| cat -e \| cat -e \| ... (10x)` | Makefile with `$$$$$$$$$$` appended to each line | 0 | ✅ | | ||
| | [✅] `yes yes \| head -n 10000000 \| wc` | `10000000 10000000 40000000` | 0 | ✅ | | ||
| | [X] `echo \| echo \| echo \| ... (13x)` then `./minishell` | ` ` | 0 | ✅ | |
| | [O] `.` | `minishell> ` | … | ✅ | | ||
| | [✅] `/` | `minishell: /: Is a directory` | 126 | ✅ | | ||
| | [X] `file/` | `file/: Not a directory` | 126 | ✅ | | ||
| | [O] `touch file_test` + `chmod +x file_test` + `./file_test` | Must not leak or leave open fds | … | ✅ | |
| typedef struct s_pid | ||
| { | ||
|
|
||
| } t_pid; |
There was a problem hiding this comment.
Thank you for your effort. Here are my notes:
- Let's keep each branch with a separate responsibility, and keep the main branch for important or final changes only.
- We need to solve the PID array issue.
- There's an extra file in the repository
- Lets keep a good commit message history please ;)
| typedef struct s_pid | ||
| { | ||
|
|
||
| } t_pid; |
There was a problem hiding this comment.
We should define what this struct is and what it includes before merging it. Or decide if we need it at all
| This is the beginning of this journey ! | ||
| https://youtu.be/PIb2aShU_H4 No newline at end of file | ||
| # minishell | ||
|
|
There was a problem hiding this comment.
We can take our time with the README.md, this works as a placeholder for now, but it should be complete before merging it to the main branch.
| @@ -0,0 +1,177 @@ | |||
| # Minishell Edge Cases | |||
There was a problem hiding this comment.
Having a tests file is a good idea, but i don't think it should be merged to the main branch.
can you check this update I made on the readme.md and added a tests.md file