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

Add support for algorithm packages, such as algorithmic and algpseudocodex [Develop branch] #498

Closed
wants to merge 4 commits into from

Conversation

Saltsmart
Copy link
Contributor

@Saltsmart Saltsmart commented Dec 4, 2023

Reopen of another pull request #496, but from the dev branch. Here is the improvement for this pull request:

It looks you might have branched from an earlier version; can you rebase from V3.23.4? you'll see that, for example, Version.pm is out of date on your branch

Here is the dev branch from V3.23.4.

the lookForThis: 1 is not necessary, so can you remove it in each occurrence?

Thanks for your advice! I've removed it.

can you add dedicated test cases that demonstrate these to test-cases/ specials? this is really important so that when new features are added/developed, I can ensure that nothing breaks.

I've added a file test-cases/specials/algorithmic-and-algpseudocodex.tex. Please review below.

This line shouldn't be the default. Please leave this value at 0

I have tried specialBeforeCommand: 0 and found this couldn't format the code. Here is the discussion.

what is this pull request about?

This pull request adds the support for algorithm packages, such as algorithmic and algpseudocodex

does this relate to an existing issue?

Here is the link: issue 427

does this change any existing behaviour?

yes

what does this add?

Here is a minimal example:

\begin{algorithm}[ht]
    \caption{some caption.}
    \begin{algorithmic}[1]
        \Require{aaa}
        \Ensure{bbb}
        % for-loop
        \For{$n = 1, \dots, 10$}
        \State body
        \EndFor
        % nesting for-loop
        \FOR{for 1}
        \FOR{for 2}
        \FOR{for 3}
        \STATE{some statement.}
        \ENDFOR
        \ENDFOR
        \ENDFOR
        % while-loop in if-statement
        \If{$quality\ge 9$}
        \State $a\gets perfect$
        \ElsIf{$quality\ge 7$}
        \State $a\gets good$
        \ElsIf{$quality\ge 5$}
        \State $a\gets medium$
        \ElsIf{$quality\ge 3$}
        \State $a\gets bad$
        \Else
        \While{$i\le n$}
        \State $sum\gets sum+i$
        \State $i\gets i+1$
        \EndWhile
        \EndIf
        % nesting blocks
        \ForAll{$n \in \{1, \dots, 10\}$}
        \State body
        \Loop
        \State body
        \EndLoop
        \State $sum\gets 0$
        \State $i\gets 1$
        \Repeat
        \State $sum\gets sum+i$
        \State $i\gets i+1$
        \Until{$i>n$}
        \EndFor
        % fuction block
        \Function{Euclid}{$a,b$}\Comment{The g.c.d. of a and b}
        \State $r\gets a\bmod b$
        \While{$r\not=0$}\Comment{We have the answer if r is 0}
        \State $a\gets b$
        \State $b\gets r$
        \State $r\gets a\bmod b$
        \EndWhile
        \State \textbf{return} $b$\Comment{The gcd is b}
        \EndFunction
    \end{algorithmic}
\end{algorithm}

After merging the pull request, latexindent.pl could format the file as follows (with 4-space indents):

\begin{algorithm}[ht]
    \caption{some caption.}
    \begin{algorithmic}[1]
        \Require{aaa}
        \Ensure{bbb}
        % for-loop
        \For{$n = 1, \dots, 10$}
            \State body
        \EndFor
        % nesting for-loop
        \FOR{for 1}
            \FOR{for 2}
                \FOR{for 3}
                    \STATE{some statement.}
                \ENDFOR
            \ENDFOR
        \ENDFOR
        % while-loop in if-statement
        \If{$quality\ge 9$}
            \State $a\gets perfect$
        \ElsIf{$quality\ge 7$}
            \State $a\gets good$
        \ElsIf{$quality\ge 5$}
            \State $a\gets medium$
        \ElsIf{$quality\ge 3$}
            \State $a\gets bad$
        \Else
            \While{$i\le n$}
                \State $sum\gets sum+i$
                \State $i\gets i+1$
            \EndWhile
        \EndIf
        % nesting blocks
        \ForAll{$n \in \{1, \dots, 10\}$}
            \State body
            \Loop
                \State body
            \EndLoop
            \State $sum\gets 0$
            \State $i\gets 1$
            \Repeat
                \State $sum\gets sum+i$
                \State $i\gets i+1$
            \Until{$i>n$}
        \EndFor
        % fuction block
        \Function{Euclid}{$a,b$}\Comment{The g.c.d. of a and b}
            \State $r\gets a\bmod b$
            \While{$r\not=0$}\Comment{We have the answer if r is 0}
                \State $a\gets b$
                \State $b\gets r$
                \State $r\gets a\bmod b$
            \EndWhile
            \State \textbf{return} $b$\Comment{The gcd is b}
        \EndFunction
    \end{algorithmic}
\end{algorithm}

how do I test this?

You can test this as the above section depicts.

anything else?

No more further comments. Best wishes for you and your family!

@cmhughes
Copy link
Owner

cmhughes commented Dec 5, 2023

Thanks this is looking good. A couple more comments from me:

  • can you update the script test-cases/specials/specials-test-cases.sh accordingly?
  • can you change specialBeforeCommand: to be 0

@Saltsmart
Copy link
Contributor Author

Sorry for not familiar with GitHub Actions!
I could see some failed tests (even they are still running now). Are they related with my pull request? Please let me know if I could help improve these tests.

@cmhughes
Copy link
Owner

cmhughes commented Dec 7, 2023

Thanks for this.

Thinking about this in more detail, I don't get why it needs to be part of the default settings. I can see why some users want it as part of their individual settings, in which case, they should add it to their individual global settings, but having it as part of default does not feel appropriate.

Can I recommend that you change this so that it simply adds a test case?

As another option, if you'd like to add an example to the documentation, please feel free.

My apologies for the back & forth.

@Saltsmart Saltsmart deleted the branch cmhughes:develop December 12, 2023 02:44
@Saltsmart Saltsmart closed this Dec 12, 2023
@Saltsmart Saltsmart deleted the develop branch December 12, 2023 02:44
@Saltsmart Saltsmart restored the develop branch December 12, 2023 02:44
@Saltsmart
Copy link
Contributor Author

Thanks for this.

Thinking about this in more detail, I don't get why it needs to be part of the default settings. I can see why some users want it as part of their individual settings, in which case, they should add it to their individual global settings, but having it as part of default does not feel appropriate.

Can I recommend that you change this so that it simply adds a test case?

As another option, if you'd like to add an example to the documentation, please feel free.

My apologies for the back & forth.

I feel very grateful for your patience. Here is a new pull request to add the example to the documentation #500. ♥

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.

2 participants