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

Changed \newcommand for \providecommand to avoid problems with latex-… #3

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

susuhahnml
Copy link
Contributor

…class-ceurart

@tortinator
Copy link
Collaborator

Hallo @susuhahnml , I guess it's a reasonable fix but perhaps its time do redesign our macros rather than patching them.
Eg. we could use \providecommand in general ... but I have to refresh my memories on the differences.
And we should check with @rkaminsk , what he thinks.

@tortinator tortinator self-requested a review July 10, 2021 14:38
@rkaminsk
Copy link
Contributor

rkaminsk commented Jul 10, 2021

It might actually be better to see that there is a redefinition. Because the name clash might be due to completely different commands. next is also a pretty generic name. Maybe temporalnext or similar might be better to avoid clashes. I also tend to start commands with an upper case letter. This avoids clashes with a lot of packages.

One can also manually workaround such issues with \let, \undef, \def, and co.

Edit: Here some reference: https://tex.stackexchange.com/questions/18541/two-conflicting-packages-in-one-document-doable.

@tortinator
Copy link
Collaborator

tortinator commented Jul 11, 2021

After reading the stackexchange, I would prefer name clashes and fixes rather than an implicit solution.

However, adding \let\next\relax after \newcommand{\next}{...} throws an error though I hoped this would simply undo the definition.
Any idea?

BTW, this is not the only clash, viz \C is next 😞

@rkaminsk
Copy link
Contributor

rkaminsk commented Jul 11, 2021

However, adding \let\next\relax after \newcommand{\next}{...} throws an error though I hoped this would simply undo the definition.

Have you tried \let\next\undefined?

EDIT: The etoolbox package also has an \undef command.

@tortinator
Copy link
Collaborator

However, adding \let\next\relax after \newcommand{\next}{...} throws an error though I hoped this would simply undo the definition.

Have you tried \let\next\undefined?

Yes, this was even my first choice 😞

EDIT: The etoolbox package also has an \undef command.

Yeah, but this would buy me another package

@tortinator
Copy link
Collaborator

After all, it might be easier to offer all these commands via \providecommand...

@rkaminsk
Copy link
Contributor

rkaminsk commented Jul 11, 2021

Just be aware that you won't be able to use them if you use the "wrong" packages. Better rename the commands instead of using \providecomand.

@tortinator
Copy link
Collaborator

Just be aware that you won't be able to use them if you use the "wrong" packages.

I guess, we had to look at our macros as convenience packages and also load them last

@rkaminsk
Copy link
Contributor

Just be aware that you won't be able to use them if you use the "wrong" packages.

I guess, we had to look at our macros as convenience packages and also load them last

I would not do it like this. But it's your project... 😄

@rkaminsk
Copy link
Contributor

We could use the prefix \ASP<command> to avoid such things.

@tortinator
Copy link
Collaborator

Just be aware that you won't be able to use them if you use the "wrong" packages. Better rename the commands instead of using \providecomand.

Well I just checked we offer 233 macros, and renaming them all would need a unique prefix like potassco or whatever
Hnce I'd sitll go for \providecommand as we do in comments.sty already

@rkaminsk
Copy link
Contributor

Just be aware that you won't be able to use them if you use the "wrong" packages. Better rename the commands instead of using \providecomand.

Well I just checked we offer 233 macros, and renaming them all would need a unique prefix like potassco or whatever
Hnce I'd sitll go for \providecommand as we do in comments.sty already

I would do exactly this. Since you are using submodules, you can just do it. It won't break any existing projects.

@susuhahnml
Copy link
Contributor Author

susuhahnml commented Jul 11, 2021 via email

@rkaminsk
Copy link
Contributor

Here is one workaround. I also tried to patch the ceurart but my TeX knowledge is a bit lacking to do this kind of stuff.

\documentclass{ceurart}

\let\oldnext\next
\let\next\undefined
\include{dtel}
\let\ASPnext\next
\let\next\oldnext

\begin{document}
\begin{itemize}
\item \(\ASPnext\) test1
\item test2
\end{itemize}
\end{document}

@susuhahnml
Copy link
Contributor Author

This fixes the clash, with \input insted of \include tho. :) But overall is probably a good option while we discuss how to tackle this problem in general.

@rkaminsk
Copy link
Contributor

This fixes the clash, with \input insted of \include tho. :) But overall is probably a good option while we discuss how to tackle this problem in general.

Sorry, I always mix this up when writing things from scratch. It has to be \input, \include is invalid in the preamble.

PS: Since the macros only contain commands, they could actually become a style.

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.

3 participants