Man Command by dtruong33 · Pull Request #84 · USF-OS/FogOS · GitHub
Skip to content

Man Command#84

Open
dtruong33 wants to merge 10 commits into
USF-OS:mainfrom
dtruong33:dev
Open

Man Command#84
dtruong33 wants to merge 10 commits into
USF-OS:mainfrom
dtruong33:dev

Conversation

@dtruong33

@dtruong33 dtruong33 commented Sep 24, 2024

Copy link
Copy Markdown

Implemented man command and framework for adding man pages. Input a command or function's name into the command to bring up its man page, automatically finding file if it exists and automatically applying the file extension. Additionally can use the -f flag to find a specific word.

Changed Files:

  • user/man.c: Man command itself
  • Makefile: Recognizes man.md
  • man.md: Example man page
  • mkfs.c: Allows docs directory to be made

Flags:

  • (-f): find specific word in man page

Limitations:

  • No scrolling, prints the entire man page at once
  • Find only prints out the lines in which the found word is contained

@rick2244

Copy link
Copy Markdown

@chansrinivas

Copy link
Copy Markdown

I can code review this

@dtruong33

Copy link
Copy Markdown
Author

Updated man command with support for using docs as a directory to hold man pages as well as various bug fixes and updating

@rick2244

Copy link
Copy Markdown

I don't see man.c in user

@jlazaro2

Copy link
Copy Markdown

i can also code review this

@chansrinivas

chansrinivas commented Oct 1, 2024

Copy link
Copy Markdown

I think your project is great and really useful! The features outlined in the issue are completed. One of the limitations is the scrolling feature which was earlier mentioned in the issue. The man page is printed all at once but it looks good this way too! For flags, I would say it's a good idea to add a "usage" message if the -F flag isn't passed in correctly as args to the program. Everything else looks good, javadoc comments are included as well! Another suggestion, it would be a good idea to have a couple more files in the docs folder for other common commands like cat, ls, cd and so on.

High Level Checks:

  • PR fully resolves the task/issue
  • Does the PR have tests?
  • Does the PR follow project code formatting standards?
  • Does the OS still compile and run?
  • Is documentation included?

Code Checks:

  • Does the code achieve its intended purpose?
  • Were all edge cases considered?
    mkfs.c and Makefile are modified to add new docs in the xv6 system. I think having more tests is a good idea. Right now,
    your docs folder only has man.md so only that can be tested.
    Perhaps having sample man pages for commands like LS and CD would be a good idea so that it could be tested without
    needing to modify the mkfs.c/Makefile to add more documents to the file system.
  • Is the code organized and maintainable?
  • Does the code maintain the same level of performance? (no performance regressions)

@jlazaro2

jlazaro2 commented Oct 3, 2024

Copy link
Copy Markdown

This man command does not only work well but prints out the information smoothly. I like how you added a flag to search up specific words and I do believe that your code is both readable and formatted well. While this pull request does meet all the issue goals, it also adds more with the -f flag.
Here's a couple of suggestions:

  • When running just the command "man" without any arguments, it prints the command page still. Instead, I suggest the terminal printing a statement asking for a specific command
  • Similar to the first suggestion, when I run "man man -f", I would suggest printing a statement asking for a specific word
  • While the -f flag achieves it's purpose, I feel like it would be better to search and receive specific information in the command pages instead. For example, if you run "man man -f AUTHOR" it would print out, "AUTHOR: " including the line in which this information is found.

High Level Checks:

  • PR fully resolves the task/issue
  • Does the PR have tests?
    The command man is tested when "man man" is run on qemu. However, there
    were no specific test files stated. I was also unsure how to test the -f flag at
    first until I looked at the examples in the man.md file, so i can assume the command is
    run like "man -f . Still, it worked once I was able to run
    properly so good job!
  • Does the PR follow project code formatting standards?
  • Does the OS still compile and run?
  • Is documentation included?

Code Checks:

  • Does the code achieve its intended purpose?
  • Were all edge cases considered?
    Not exactly, see suggestions up above!
  • Is the code organized and maintainable?
  • Does the code maintain the same level of performance? (no performance regressions)

@malensek

Copy link
Copy Markdown
Contributor

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.

5 participants