david-yz-liu / memory-viz

Javascript library for creating beginner-friendly memory model diagrams.
https://www.cs.toronto.edu/~david/memory-viz/demo/
2 stars 7 forks source link

Adding CLI Streams #52

Closed sarahsonder closed 3 months ago

sarahsonder commented 3 months ago

Proposed Changes

This PR allows for new input and output streams to the CLI. Specifically, the <file-to-path> argument was made optional, allowing users to also use stdin as input if a file path is not provided. Users can also redirect output to a location of their choice using --output<path> or print the SVG directly to standard output.

Type of Change

Type Applies?
🚨 Breaking change (fix or feature that would cause existing functionality to change) X
✨ New feature (non-breaking change that adds functionality) X
πŸ› Bug fix (non-breaking change that fixes an issue)
🎨 User interface change (change to user interface; provide screenshots)
♻️ Refactoring (internal change to codebase, without changing functionality)
🚦 Test update (change that only adds or modifies tests)
πŸ“š Documentation update (change that only updates documentation)
πŸ“¦ Dependency update (change that updates a dependency)
πŸ”§ Internal (change that only affects developers or continuous integration)

Checklist

(Complete each of the following items for your pull request. Indicate that you have completed an item by changing the [ ] into a [x] in the raw text, or by clicking on the checkbox in the rendered description on GitHub.)

Before opening your pull request:

After opening your pull request:

Questions and Comments

Hi Professor Liu! So far, I have made the --filepath argument optional and I have also added the options --stdout, --output, and --stdin. However, I have a few questions about the --stdin option.

  1. As a general sanity check, are my current changes in the right direction?
  2. When using standard input, how exactly do you want users to give input? For example, in the terminal, would you want users to enter npx memory-viz --stdin, press enter, type in the json, and then press enter again to run memory-viz? I'm still working on getting input directly from the terminal, but I can currently get the command Write-Output '[{ "type": "int", "id": 13, "value": 7}]'| npx memory-viz --stdin --stdout to work. For reference, I'm using Windows, hence the Write-Output command.
  3. For --stdin, what would you like the output file to be named? I currently have it named as memory_viz.svg
  4. In your slack message, you mentioned that if --stdin is chosen, there's won't be a default path. However, I believe that the default path is wherever memory-viz is being called because Write-Output '[{ "type": "int", "id": 13, "value": 7}]'| npx memory-viz --stdin outputs into my memory-viz folder. That being said, I could be mistaken and this might be different be for users.

Thank you so much!

david-yz-liu commented 3 months ago

@sarahsonder good progress.

  1. Yes you're on the right track.
  2. "reading from stdin" means "read fully from stdin". You learned about various approaches for this in CSC209, and you're correct that one approach is to use a pipe. I think your main confusion lies in that you aren't aware that when just typing in input, the user can use a keyboard shortcut to signal the end of their input. This is likely Ctrl+D for you. In other words, the "Enter" key should not be treated in any special way; instead, the user should be able to type in as much JSON as they like, and then signal the end of input with a keyboard shortcut. 3 and 4. The term "path" includes both directory and filename. Please re-read my instructions with this in mind.
coveralls commented 3 months ago

Pull Request Test Coverage Report for Build 10002435167

Details


Totals Coverage Status
Change from base Build 9855023876: 0.0%
Covered Lines: 407
Relevant Lines: 438

πŸ’› - Coveralls