Skip to content

[CS2113-W13-1] ZettelCLI#41

Open
shauntsr wants to merge 609 commits into
nus-cs2113-AY2526S1:masterfrom
AY2526S1-CS2113-W13-1:master
Open

[CS2113-W13-1] ZettelCLI#41
shauntsr wants to merge 609 commits into
nus-cs2113-AY2526S1:masterfrom
AY2526S1-CS2113-W13-1:master

Conversation

@shauntsr
Copy link
Copy Markdown

@shauntsr shauntsr commented Oct 4, 2025

ZettelCLI is a desktop Command Line Interface (CLI) application used for managing a personal Zettlekesten note system. It is optimised for users who prefer keyboard commands over Graphical User Interfaces (GUI). Through just a keyboard, users can build their own repository of notes or quickly navigate through their knowledge base with ease.

danielkwan2004 pushed a commit to danielkwan2004/tp that referenced this pull request Oct 15, 2025
gordonajajar and others added 18 commits October 28, 2025 16:47
For example, if attempting to open vim from IntelliJ Idea's Run, a
pseudoconsole
…tadata, added noteserializer test and jdocs for storage classes
Feature: add deleteNoteCommand functionality to delete body file + me…
…directions

Feature: Unlink any 2 notes in both directions with 1 command
@andrewcai8
Copy link
Copy Markdown

Missing activation bar
image

@andrewcai8
Copy link
Copy Markdown

This diagram doesn't seem to be correct. Does not follow sequence diagram rules.
image

@andrewcai8
Copy link
Copy Markdown

Sequence diagram missing activate bars, and should add left arrows to show return of control. Would recommend using PlantUML to draw these diagrams.

image

@andrewcai8
Copy link
Copy Markdown

The class diagram and the key fields section is a bit redundant, as the class attributes gets repeated twice.

image

Copy link
Copy Markdown

@duckyfuz duckyfuz left a comment

Choose a reason for hiding this comment

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

Would it be good to add instructions for manual testing?

Comment thread docs/DeveloperGuide.md Outdated
├─ Read User Input (with timeout)
├─ Parse Command
├─ Execute Command
├─ Auto-save Notes
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I see Auto-save Notes is listed as a step inside the main loop. Does this imply it runs after every single command, even read-only ones?

Comment thread docs/DeveloperGuide.md Outdated
- Handle graceful shutdown and error recovery
- Auto-save notes after each command

**Main Application Flow:**
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Have you considered using a tool like PlantUML (puml) to generate this diagram instead of typing it out manually?

It might make it much easier to maintain, version control alongside your code, and embed directly into documentation (like in a README.md). For a process flow like this, you could use an activity diagram, which would also make modeling the loop and any conditional logic explicit.

Comment thread docs/DeveloperGuide.md Outdated
Comment on lines +64 to +77
**Class Diagram:**
```
┌─────────────────────────┐
│ Zettel │
├─────────────────────────┤
│ - storage: Storage │
│ - notes: ArrayList │
│ - ui: UI │
│ - isRunning: boolean │
├─────────────────────────┤
│ + run(): void │
│ + main(String[]): void │
└─────────────────────────┘
```
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Just like with the activity diagram, have you considered generating this class diagram with PlantUML? It's excellent for keeping your diagrams in sync with your code, as you can just edit the text source.

This syntax also makes it very easy to add relationship lines, like associations to the Storage and UI classes, or composition if Zettel "owns" the notes.

Comment thread docs/DeveloperGuide.md Outdated
Comment on lines +314 to +341
Input: pin a1b2c3d4
[Validation 1] Format Check
├─ Length = 8? ✓
├─ Matches [a-f0-9]? ✓
└─ Pass → Continue
[Validation 2] Empty List Check
├─ notes.isEmpty()?
├─ If true → throw NoNotesException
└─ If false → Continue
[Validation 3] Note Existence Check
├─ Find note with ID
├─ If not found → throw InvalidNoteIdException
└─ If found → Continue
[Validation 4] State Check
├─ Is note.isPinned() == isPin?
├─ If true → throw AlreadyPinnedException
└─ If false → Continue
[Happy Path] Execute Operation
├─ Get note from Optional
├─ Set pinned status
├─ Display confirmation
└─ Save to storage
```
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

As with the other diagrams, have you considered using a PlantUML activity diagram?

It's particularly good at showing this kind of "guard clause" or "fail-fast" pattern, where each if statement acts as a gate that the logic must pass through.

This way, the different paths (the happy path vs. the various error exceptions) can all be visualized very clearly.

Comment thread docs/DeveloperGuide.md Outdated
Comment on lines +388 to +414
**Sequence Diagram - Pin Note Operation:**

```
User → Parser → PinNoteCommand → Notes → Note → UI → Storage
| | | | | | |
| pin a1b2c3d4 | | | | |
|------>| | | | | |
| | new PinNoteCommand | | | |
| |----------->| | | | |
| | | | | | |
| | execute()| | | | |
| |----------->| | | | |
| | | validateFormat() | | |
| | |---- | | | |
| | | | | | | |
| | |<---- | | | |
| | | isEmpty()? | | | |
| | |----------->| | | |
| | |<-----------| | | |
| | | stream() | | | |
| | | filter() | | | |
| | | findFirst()| | | |
| | |----------->| | | |
| | |<-----------| | | |
| | | | | |
| | | get() | | |
| | |------------------>| | |
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Same here, have you considered using a UML diagram?

Additionally, should there be arrows pointing from class to class?
eg. in the first line
User → Parser → PinNoteCommand → Notes → Note → UI → Storage

Comment thread docs/DeveloperGuide.md Outdated
Comment on lines +553 to +579
**Class Diagram:**
```
┌─────────────────────────────────┐
│ Note │
├─────────────────────────────────┤
│ - id: String (8 chars) │
│ - title: String │
│ - filename: String │
│ - body: String │
│ - createdAt: Instant │
│ - modifiedAt: Instant │
│ - pinned: boolean │
│ - archived: boolean │
│ - archiveName: String │
│ - logs: List<String> │
│ - numberOfNotes: int (static) │
├─────────────────────────────────┤
│ + Note(id, title, ...) │
│ + getId(): String │
│ + getTitle(): String │
│ + setTitle(String): void │
│ + setPinned(boolean): void │
│ + addLog(String): void │
│ + updateModifiedAt(): void │
│ + toString(): String │
└─────────────────────────────────┘
```
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Similarly, have you considered using a UML diagram for this?

Perhaps showing all classes in a overview UML diagram, then "zooming in" on each class would provide a clearer view of how the classes relate to each other.

Comment thread docs/DeveloperGuide.md Outdated
│ - logs: List<String> │
│ - numberOfNotes: int (static) │
├─────────────────────────────────┤
│ + Note(id, title, ...) │
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

What other params does the constructor take in?

Additionally, for overloaded methods, would it be better to list each overloaded operation individually instead of using ellipsis?

Perhaps a UML diagram would be better here, as each line can be longer without you having to worry about the formatting of the box.
I see that the Note constructor takes in many params.

Comment thread docs/DeveloperGuide.md Outdated
│ - archived: boolean │
│ - archiveName: String │
│ - logs: List<String> │
│ - numberOfNotes: int (static) │
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Is this the correct notation for indicating static methods?

Comment thread docs/DeveloperGuide.md Outdated
- All I/O operations wrapped in try–catch
- On failure: print readable warning (no crash)
- Missing folders/files are auto-created
- Corrupted index entries are skipped (with console notice)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Regarding the "Corrupted index entries are skipped": When a corrupted entry is found and skipped, is it just for that one session? Or does your app perhaps quarantine it or mark it as "corrupted" so it doesn't try to parse it (and log a notice) every single time the app starts?

Comment thread docs/DeveloperGuide.md Outdated
│ - logger: Logger │
├─────────────────────────────────┤
│ + PinNoteCommand(String, bool) │
│ + execute(ArrayList, UI, ...) │
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

What params does execute take in?

danielkwan2004 and others added 30 commits November 4, 2025 11:02
Fix: init validation and tests
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.

10 participants